fix(nav): clamp dropdown to viewport and portal into fullscreen root#10297
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Architecture diagram
sequenceDiagram
participant Cell as Cell Output Area
participant Trigger as Nav Menu Trigger
participant Viewport as NavigationMenuViewportPortal
participant Dropdown as Dropdown Content
participant Portal as NavigationMenuPortal
participant FS as Fullscreen Root
participant Body as document.body
Note over Trigger,Viewport: Menu opens via hover/click
Trigger->>Viewport: open=true
Viewport->>Viewport: Measure anchor rect
Viewport->>Viewport: Compute contentWidth from ref
alt Content wider than viewport
Viewport->>Viewport: Clamp left to VIEWPORT_MARGIN
Viewport->>Viewport: Clamp left to maxLeft (avoid overflow)
end
Viewport->>Viewport: setPosition({left, top})
alt Fullscreen active
Viewport->>Portal: Render into fullscreen root
Portal->>FS: createPortal(children, fullscreenEl)
Note over Dropdown,FS: Position: fixed, z-50 inside fullscreen context
else No fullscreen
Viewport->>Portal: Render into document.body
Portal->>Body: createPortal(children, document.body)
end
Dropdown-->>Trigger: Dropdown visible
Viewport->>Viewport: requestAnimationFrame(updatePosition)
Note over Viewport: Re-measure after layout to account for shifts
alt User clicks trial link
Trigger->>Dropdown: link.click({ trial: true })
Dropdown-->>Cell: Playwright hit-target check
alt Link clipped by cell overflow
Cell-->>Dropdown: Test fails (not visible)
else Link portaled & unclipped
Cell-->>Dropdown: Test passes (visible)
end
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
Coverage Report for ./frontend
File Coverage
|
||||||||||||||||||||||||||||||||||||||
The kitchen_sink notebook declares auto_instantiate in its script metadata, which is now ignored for security, so its cells never run in edit mode and the nav menu output is never rendered — the trigger could never be found. Move the test to a minimal, dependency-free notebook served in run mode, which always instantiates server-side.
Comment on lines
153
to
+157
| React.useLayoutEffect(() => { | ||
| updatePosition(); | ||
| }, [updatePosition]); | ||
| if (open) { | ||
| const raf = requestAnimationFrame(updatePosition); | ||
| return () => cancelAnimationFrame(raf); |
Contributor
|
🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.23.15-dev66 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.