fix(super-editor): flip tooltip when clipped by viewport#3836
fix(super-editor): flip tooltip when clipped by viewport#3836dani-beltran wants to merge 6 commits into
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0967f00061
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| case 'top-start': | ||
| top = triggerRect.top - contentHeight - offset; |
There was a problem hiding this comment.
When a bottom-start dropdown flips upward, this subtracts the full content height before callers apply the new maxHeight; for example the existing long-menu case with a 500px menu, trigger top=260, and only 248px available above computes top=-244px, then renders a 248px-tall menu mostly offscreen. Use the constrained render height or clamp to the gutter as the previous dropdown code did, otherwise long toolbar/font dropdowns near the bottom still get clipped after flipping.
Useful? React with 👍 / 👎.
| .fade-in-scale-up-transition-enter-active, | ||
| .fade-in-scale-up-transition-leave-active { | ||
| transform-origin: bottom center; | ||
| transform-origin: center; |
There was a problem hiding this comment.
I changed the animation origin to be position agnostic, since the tooltip could be now on top or bottom positions. It looks good, barely noticeable difference. However, this is modifying the original animation, so I am considering changing it.
… outside viewport
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7b4b2b2a9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const { placement = 'top', offset = 0, flip = true } = options; | ||
| const triggerRect = triggerElem.getBoundingClientRect(); | ||
| const contentWidth = contentElem.offsetWidth; | ||
| const contentHeight = contentElem.offsetHeight; |
There was a problem hiding this comment.
For toolbar/font dropdowns, menuPosition.maxHeight is kept in component state after a close, so the element created on the next open can already be constrained by the previous max-height. Measuring offsetHeight here then uses the clipped height rather than the full menu height; for example a 500px menu reopened near the bottom with a stale 120px maxHeight and 200px below/400px above will no longer flip upward, leaving the long menu clipped below. The previous inline dropdown logic used scrollHeight for this decision, so this should measure the scrollable height with an offset fallback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Sure, but using scrollHeight instead of offsetHeight will mess up content components that are not dropdowns. I will add some way of detecting that is a dropdown or add a parameter in the function to indicate it is.
1 participant
Fixes #3786
Tooltip positioning wasn't detecting viewport collisions, it only clamped horizontally, so tooltips could still overflow on top.
This PR fixes the issue, while also refactoring some other components using the same duplicated logic. Also created a repro test for the bug.
Refactor notes:
FontFamilyCombobox'salignment behaviorToolbarDropdownDemo of how FontFamilyCombobox and ToolbarDropdown components behave after the change. They retain the same behavior as before. 👇