Skip to content

fix nav menu dropdown clipping#9469

Open
ralphptorres wants to merge 6 commits into
marimo-team:mainfrom
ralphptorres:fix-nav-clipping
Open

fix nav menu dropdown clipping#9469
ralphptorres wants to merge 6 commits into
marimo-team:mainfrom
ralphptorres:fix-nav-clipping

Conversation

@ralphptorres
Copy link
Copy Markdown
Contributor

@ralphptorres ralphptorres commented May 7, 2026

use portaled nav dropdowns to resolve clipping in notebook cells while supporting hover, click, and keyboard interactions

closes #7167

allow output area overflow on hover/focus to prevent clipping, and raise
z-index to 50 for better stacking
Copilot AI review requested due to automatic review settings May 7, 2026 14:16
@vercel
Copy link
Copy Markdown

vercel Bot commented May 7, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
marimo-docs Ready Ready Preview, Comment May 10, 2026 8:50am

Request Review

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 7, 2026

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="frontend/src/css/app/Cell.css">

<violation number="1" location="frontend/src/css/app/Cell.css:100">
P2: `overflow: visible !important` will override the table-specific `overflow: hidden` (line ~109) on hover/focus, since `!important` trumps specificity. Consider scoping the rule to exclude the table case, e.g. using `:not(:has(> .output:only-child > marimo-ui-element:only-child > marimo-table))`, or adding a matching `!important` to the table rule to preserve its override.</violation>
</file>
Architecture diagram
sequenceDiagram
    participant User as User
    participant Nav as NavigationMenu
    participant Cell as Notebook Cell
    participant Output as Output Area
    participant Stack as Z-Index Context
    
    Note over User,Stack: Nav Menu Dropdown Display Flow
    
    User->>Nav: Hover/focus nav menu trigger
    Nav->>Nav: Check orientation & visibility
    Nav->>Stack: Set z-index to 50
    Stack-->>Nav: Elevated stacking
    
    Note over Nav,Cell: Menu dropdown now stacks above cell
    
    User->>Cell: Scroll or interact with notebook
    Cell->>Cell: hover output area
    
    alt Output area has content overflow
        Cell->>Output: NEW: overflow: visible (on hover/focus-within)
        Output->>Output: Display full content without clipping
        Output-->>Cell: Content visible
    else Output area no overflow
        Output->>Output: Normal overflow behavior
    end
    
    Note over Cell,Output: Previously: overflow was hidden/clipped
    
    Cell->>Stack: Maintain default z-index context
    Stack-->>Nav: Nav dropdown remains on top (z=50)
Loading

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.

Comment thread frontend/src/css/app/Cell.css Outdated
@ralphptorres
Copy link
Copy Markdown
Contributor Author

ralphptorres commented May 7, 2026

I have read the CLA Document and I hereby sign the CLA

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@ralphptorres
Copy link
Copy Markdown
Contributor Author

recheck

Comment thread frontend/src/css/app/Cell.css Outdated
ralphptorres and others added 3 commits May 10, 2026 08:28
reverts fragile clipping fix from commits 5499888 and cf5db99
use portaled Popover components for horizontal menus in
NavigationMenuPlugin. this prevents clipping while maintaining hover
behavior and left-alignment
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 issues found across 3 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="frontend/src/css/app/Cell.css">

<violation number="1">
P2: Removing the hover/focus overflow override regresses interactive outputs by keeping `.output-area` clipped (`overflow: auto`) even when menus/popovers need to extend outside the cell.</violation>
</file>

<file name="frontend/src/plugins/layout/NavigationMenuPlugin.tsx">

<violation number="1" location="frontend/src/plugins/layout/NavigationMenuPlugin.tsx:135">
P2: Controlled popover ignores open requests, so the dropdown cannot open via click/keyboard/touch and becomes hover-only.</violation>
</file>

<file name="frontend/src/components/ui/navigation.tsx">

<violation number="1">
P2: Lowering the navigation menu root from z-50 to z-10 regresses the dropdown’s stacking order and can put it behind surrounding UI, undermining the clipping fix.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Comment thread frontend/src/plugins/layout/NavigationMenuPlugin.tsx Outdated
ensures nav menu can be opened via click and keyboard
@ralphptorres
Copy link
Copy Markdown
Contributor Author

ralphptorres commented May 10, 2026

@mscolnick thanks for the suggestion, applied it!

quick question: this dropdown currently opens on hover, while most other buttons in the ui open (and close) dropdowns on click. should i align this behavior? it's outside this commit's scope but we're already in these files

@mscolnick
Copy link
Copy Markdown
Contributor

i think this changes more than i'd expect. ideally there is a NavigationPortal (open issue here: radix-ui/primitives#3159). maybe in the meantime we can keep the navigation components and maybe create our own <NavigationPortal (example here: radix-ui/primitives@c95104b#diff-8e96b8f4b466ef7144a8f478a3e0c8afcecc7f42ab59b0af0595889df64f1c28R1105)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Popup menus not appearing correctly

3 participants