Skip to content

[EXPERIMENT] Nav hover interaction#109621

Open
itsdangold wants to merge 5 commits intomasterfrom
nav-hover
Open

[EXPERIMENT] Nav hover interaction#109621
itsdangold wants to merge 5 commits intomasterfrom
nav-hover

Conversation

@itsdangold
Copy link
Contributor

Playing around with the navbar hover ux. Vibe coded to the max, proceed with caution.

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Feb 27, 2026
const defaultActiveNavGroup = useActiveNavGroup();

const activeNavGroup = activePrimaryNavGroup ?? defaultActiveNavGroup;
const activeNavGroup = useActiveNavGroup();
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: The sidebar's hover-to-preview feature is broken in collapsed mode because it now uses a route-based hook (useActiveNavGroup) instead of listening for the hover state.
Severity: CRITICAL

Suggested Fix

Modify SecondarySidebar to prioritize the activePrimaryNavGroup value from the context when it is set (i.e., on hover). It should fall back to using useActiveNavGroup() only when no item is being hovered. This can be achieved by using a construct like const activeNavGroup = activePrimaryNavGroup ?? useActiveNavGroup();.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: static/app/views/nav/secondary/secondarySidebar.tsx#L45

Potential issue: In the collapsed navigation mode, the hover-to-preview feature is
broken. The `SecondarySidebar` component was changed to use the `useActiveNavGroup()`
hook, which determines the active navigation group based on the current URL route. This
change ignores the `activePrimaryNavGroup` context value, which is specifically set when
a user hovers over a navigation item in collapsed mode. As a result, hovering over a
navigation item no longer shows a preview of its secondary menu; instead, the sidebar
continues to display the secondary menu for the current page's route.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

if (!collapsedNavIsOpen) {
setActivePrimaryNavGroup(group);
}
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Collapsed nav hover-to-switch stops working when flyout open

Medium Severity

When isCollapsed is true and collapsedNavIsOpen is also true, the new code returns early without doing anything. Previously, this case fell through to the delay/timer logic and eventually called setActivePrimaryNavGroup(group), allowing users to hover between nav group icons to switch the flyout content. Now hover-to-switch is broken in collapsed mode once the flyout is open — the user must click instead.

The comment says "keep existing flyout behavior" but the restructured guard actually changes it. The outer if (isCollapsed) { ... return; } unconditionally bails out, whereas the old if (isCollapsed && !collapsedNavIsOpen) only bailed for the not-yet-open case.

Fix in Cursor Fix in Web

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

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant