fix(rate-ui): clamp top-toolstrip popovers inside constrained viewports (#4300) - #5255
dieterolson wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b048e5a67e
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <div | ||
| ref={popover.panelRef} | ||
| style={popover.style} | ||
| className={`${POPOVER_CLASS} min-w-96`} |
There was a problem hiding this comment.
Allow the View popover to shrink on narrow screens
At the repository's existing 390 px chromium-narrow viewport (playwright.config.ts:63-68), min-w-96 forces this panel to 384 px while useViewportClampedPopover reserves a 16 px gutter on each side. The clamp therefore places its left edge at 16 px and its right edge at 400 px, leaving the View menu outside the supported viewport; the new test only exercises 520 px and is excluded from the narrow project by that project's testMatch. Make this panel responsive or relax the gutter when its width exceeds the available space, and cover the 390 px configuration.
AGENTS.md reference: AGENTS.md:L339-L343
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
rate-visual-exemption: Clamped top-toolstrip popovers inside constrained viewport bounds without altering tab layout
Fixes #4300
rate-visual-exemption: Clamped top-toolstrip popovers inside constrained viewport bounds without altering tab layout
Summary
On the Rate of Closure React surface at a 520 x 900 viewport, opening top-toolstrip menus (File, View, Tools) caused popovers to position partially outside the viewport, clipping items and text.
This PR:
useViewportClampedPopoverhook intoFileMenu,ViewMenu, andToolsMenuinAppToolstrip.tsx.ToolsMenuinto a clean component complying with Law of Demeter and max line limits.AppToolstrip.test.tsxverifying horizontal popover translation under constrained viewports.src/rate_of_closure/web/e2e/toolstrip-popover-viewport.spec.tsproving all popovers stay within the 520x900 viewport with no document horizontal overflow.