Skip to content

fix(menu): reset sticky hover state on mouse leave #1827

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abhi-7-7 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(menu): reset sticky hover state on mouse leave #1827

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions packages/fuselage/src/components/Options/Options.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,9 @@ const Options = forwardRef<HTMLElement, OptionsProps>(function Options(
}}
key={value}
value={value}
selected={selected || (multiple !== true && undefined)} // TODO: undefined?
selected={selected ? true : undefined}
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

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

This change alters the behavior from the original logic. The original code was selected || (multiple !== true && undefined), which would:

  • Set selected to true when selected is truthy
  • Set selected to undefined when selected is falsy AND multiple is true
  • Set selected to false when selected is falsy AND multiple is false (because false || false evaluates to false)

The new code selected ? true : undefined always sets it to undefined when selected is falsy, regardless of the multiple flag. This removes the distinction between single-select and multi-select mode for unselected items.

If this behavioral change is intentional, it should be documented. Otherwise, the logic should preserve the original behavior or clarify what the correct behavior should be.

Suggested change
selected={selected ? true : undefined}
selected={selected || (multiple !== true && undefined)}

Copilot uses AI. Check for mistakes.
disabled={disabled}
focus={cursor === i || undefined} // TODO: undefined?
focus={cursor === i ? true : undefined}
/>
);
}
Expand All @@ -119,6 +119,12 @@ const Options = forwardRef<HTMLElement, OptionsProps>(function Options(
maxHeight={maxHeight}
onMouseDown={prevent}
onClick={prevent}
onMouseLeave={() => {
// Force browser to recalculate hover states when mouse leaves menu
if (liRef.current) {
void liRef.current.offsetHeight;
}
}}
Comment on lines +122 to +127
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

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

The offsetHeight access is intended to force a browser reflow to clear hover states, but this approach has several issues:

  1. This is a performance anti-pattern that forces synchronous layout recalculation
  2. It doesn't reliably clear CSS :hover pseudo-class states - browser hover states are managed by the browser's event system and cannot be programmatically cleared by triggering reflows
  3. The root cause of sticky hover states is typically related to pointer events not properly propagating, or the browser not detecting mouse leave events during rapid movements or when elements are removed/repositioned

Consider alternative solutions:

  • Ensure proper CSS specificity so the focus class overrides hover states
  • Use pointer-events CSS property strategically
  • Investigate if the issue is related to event handlers preventing default behavior or stopping propagation
Suggested change
onMouseLeave={() => {
// Force browser to recalculate hover states when mouse leaves menu
if (liRef.current) {
void liRef.current.offsetHeight;
}
}}

Copilot uses AI. Check for mistakes.
is='ol'
aria-multiselectable={multiple || true}
role='listbox'
Expand Down
4 changes: 2 additions & 2 deletions packages/fuselage/src/components/OptionsPaginated/OptionsPaginated.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ export const OptionsPaginated = forwardRef<Element, OptionsPaginatedProps>(
}}
key={value}
value={value}
selected={selected || (multiple !== true && undefined)} // FIXME: undefined???
focus={cursor === index || undefined} // FIXME: undefined???
selected={selected ? true : undefined}
Copy link

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

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

Similar to the issue in Options.tsx, this change alters the original behavior. The original code selected || (multiple !== true && undefined) had different logic than the new selected ? true : undefined.

The original would return false (not undefined) for unselected items in single-select mode when multiple !== true evaluates to true (i.e., when multiple is false or undefined). The new code always returns undefined for unselected items.

Ensure this behavioral change is intentional and consistent with how the Option component expects to receive the selected prop in different modes.

Suggested change
selected={selected ? true : undefined}
selected={selected}

Copilot uses AI. Check for mistakes.
focus={cursor === index ? true : undefined}
/>
);
};
Expand Down
Loading