fix(nav): stop project navbar items clipping off-screen#7968
Conversation
The project navbar overflow logic mis-measured available width, letting items (e.g. Project Settings) clip past the right edge instead of collapsing into the overflow menu. - Read the real flex gap and subtract container padding instead of guessing; measure item widths with getBoundingClientRect so fractional widths are not lost. - Re-measure when a visible item changes size after the first pass (async permission-gated links, count badges). - Wrap rendered children so DOM nodes map 1:1 with items, and split the admin-only links into separate items so they overflow independently. Also shorten two labels: "Feature Lifecycle" -> "Lifecycle" and "Release Pipelines" -> "Pipelines". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR modifies the overflow visible-count hook to subtract container padding from the measured width, and changes OverflowNav to wrap each rendered item in a dedicated div. It also renames the project lifecycle link, changes the release pipelines label to “Pipelines”, and splits the project and organisation admin links into separate conditional render blocks. Estimated code review effort: 3 (Moderate) | ~20 minutes Comment |
The admin-only Organisation Integrations and Organisation Settings links were wrapped in a single fragment, so the overflow logic treated them as one double-width item and dropped both at once instead of hiding just the one that no longer fits. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Docker builds report
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 57889e17-5427-45a6-aaab-0ba47c41e344
📒 Files selected for processing (3)
frontend/common/hooks/useOverflowVisibleCount.tsfrontend/web/components/navigation/OverflowNav.tsxfrontend/web/components/navigation/navbars/ProjectNavbar.tsx
Debounce the ResizeObserver re-measure through requestAnimationFrame so a burst of resize entries triggers a single setWidths reset instead of one per entry, and cancel any pending frame on cleanup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Failed testsfirefox › tests/roles-test.pw.ts › Roles Tests › Roles can be created with project and environment permissions @enterprise Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Failed testsfirefox › tests/roles-test.pw.ts › Roles Tests › Roles can be created with project and environment permissions @enterprise Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (oss - depot-ubuntu-latest-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Failed testsfirefox › tests/roles-test.pw.ts › Roles Tests › Roles can be created with project and environment permissions @enterprise Details
Failed testsfirefox › tests/roles-test.pw.ts › Roles Tests › Roles can be created with project and environment permissions @enterprise Details
Failed testsfirefox › tests/roles-test.pw.ts › Roles Tests › Roles can be created with project and environment permissions @enterprise Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/common/hooks/useOverflowVisibleCount.ts (1)
56-85: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftResize tracking stops for overflowed items.
useOverflowVisibleCountonly observesitemsCont.children, andOverflowNav.tsxrenders(isMeasuring ? items : visible), so once an item moves into overflow it is unmounted from the measuring container and can no longer be watched. If async content changes size while hidden, the cached width goes stale and the next fit calculation can be wrong. Keep overflowed items mounted in the measuring container, or reattach the observer when the rendered child set changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b9642139-7a5c-4a3c-bf2b-6226a101e852
📒 Files selected for processing (1)
frontend/common/hooks/useOverflowVisibleCount.ts
Visual Regression19 screenshots compared. See report for details. |
The extra behaviours added to useOverflowVisibleCount — a per-child ResizeObserver re-measure and reading the real column-gap — changed measurement for every consumer of the hook, including the modal Tabs. That regressed the roles e2e (the Permissions tab briefly became non-interactive), so revert those and keep only the container-padding subtraction, which is what actually stopped the navbar items clipping. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
matthewelwell
left a comment
There was a problem hiding this comment.
Superficial review of the code + manual test via preview link.
docs/if required so people know about the feature.Changes
How did you test this code?
Viewed features page with different widths