fix(popover): single hover-follows highlight without stealing input focus (UAT)
Second UAT finding on the tag popover: the keyboard-active row stayed put while a fainter CSS :hover chased the pointer — two competing highlights. Root cause: floating-ui's focusItemOnHover is (correctly) OFF for input-hosting menus because setting its activeIndex on hover drags DOM focus onto the item and blurs the create-tag input mid-typing; the only pointer feedback left was the item's own `hover:bg-*`. Fix, applied in the shared PopoverItem so it's general: - Drive the visible highlight off a NEW `hoverIndex` state (highlightIndex = hoverIndex ?? activeIndex), decoupled from floating-ui's activeIndex — hovering sets only hoverIndex, never DOM focus, so one highlight follows the pointer AND the input keeps its caret. Keyboard nav (onNavigate/onMatch) clears hoverIndex so it takes the highlight back; leaving the menu clears it too. - Split PopoverItem's `active` into focusActive (roving tabindex, keyboard) vs visualActive (data-active highlight). - Remove the now-redundant `hover:bg-*` from the 5 consumers so data-active is the sole highlight (it was also a same-specificity tie with data-active). Verified in-browser: hovering moves the single highlight; typing "testtag" in the create field then hovering items keeps focus+caret on the input (caretPos intact); mouse-leave returns the highlight to the keyboard row. Gate: tsc/eslint/prettier/ vitest 79 green.
This commit is contained in:
@@ -124,7 +124,7 @@ export default function AddToPlaylist({
|
|||||||
// the floating element only). `toggle()` already no-ops while busy, so re-entrancy is
|
// the floating element only). `toggle()` already no-ops while busy, so re-entrancy is
|
||||||
// guarded without touching focus; `aria-busy` on the list conveys the pending state.
|
// guarded without touching focus; `aria-busy` on the list conveys the pending state.
|
||||||
label={playlistName(pl, t)}
|
label={playlistName(pl, t)}
|
||||||
className="w-full flex items-center gap-2 text-sm px-1.5 py-1.5 rounded-lg text-fg outline-hidden transition hover:bg-card/60"
|
className="w-full flex items-center gap-2 text-sm px-1.5 py-1.5 rounded-lg text-fg outline-hidden transition"
|
||||||
>
|
>
|
||||||
<span
|
<span
|
||||||
className={`grid place-items-center w-4 h-4 rounded border ${
|
className={`grid place-items-center w-4 h-4 rounded border ${
|
||||||
|
|||||||
@@ -109,6 +109,12 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) {
|
|||||||
const elementsRef = useRef<Array<HTMLElement | null>>([]);
|
const elementsRef = useRef<Array<HTMLElement | null>>([]);
|
||||||
const labelsRef = useRef<Array<string | null>>([]);
|
const labelsRef = useRef<Array<string | null>>([]);
|
||||||
const [activeIndex, setActiveIndex] = useState<number | null>(null);
|
const [activeIndex, setActiveIndex] = useState<number | null>(null);
|
||||||
|
// Pointer-hover highlight, kept SEPARATE from floating-ui's activeIndex on purpose: useListNavigation
|
||||||
|
// syncs DOM focus to activeIndex, so setting activeIndex on hover would blur a menu's free-text input
|
||||||
|
// (the create-tag field) mid-typing. hoverIndex drives only the VISUAL highlight; DOM focus never
|
||||||
|
// follows it. The highlighted row = hoverIndex ?? activeIndex, so there is exactly one highlight that
|
||||||
|
// tracks the pointer when hovering and the keyboard otherwise.
|
||||||
|
const [hoverIndex, setHoverIndex] = useState<number | null>(null);
|
||||||
|
|
||||||
const { refs, floatingStyles, context, middlewareData, isPositioned } = useFloating({
|
const { refs, floatingStyles, context, middlewareData, isPositioned } = useFloating({
|
||||||
nodeId,
|
nodeId,
|
||||||
@@ -137,7 +143,10 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) {
|
|||||||
// A stale active item would highlight (and steal the next open's initial focus) the second time a
|
// A stale active item would highlight (and steal the next open's initial focus) the second time a
|
||||||
// menu opens; clear it whenever the surface closes.
|
// menu opens; clear it whenever the surface closes.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!opts.open) setActiveIndex(null);
|
if (!opts.open) {
|
||||||
|
setActiveIndex(null);
|
||||||
|
setHoverIndex(null);
|
||||||
|
}
|
||||||
}, [opts.open]);
|
}, [opts.open]);
|
||||||
|
|
||||||
const click = useClick(context, { enabled: !isHover });
|
const click = useClick(context, { enabled: !isHover });
|
||||||
@@ -159,7 +168,12 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) {
|
|||||||
enabled: isNav,
|
enabled: isNav,
|
||||||
listRef: elementsRef,
|
listRef: elementsRef,
|
||||||
activeIndex,
|
activeIndex,
|
||||||
onNavigate: setActiveIndex,
|
// Keyboard (and pointer, when floating-ui owns it) navigation takes the visual highlight back from
|
||||||
|
// a stale pointer-hover position.
|
||||||
|
onNavigate: (i) => {
|
||||||
|
setActiveIndex(i);
|
||||||
|
setHoverIndex(null);
|
||||||
|
},
|
||||||
selectedIndex: opts.selectedIndex ?? null,
|
selectedIndex: opts.selectedIndex ?? null,
|
||||||
loop: true,
|
loop: true,
|
||||||
// Focus the first item on open for BOTH click and keyboard (the default 'auto' skips pointer-opens,
|
// Focus the first item on open for BOTH click and keyboard (the default 'auto' skips pointer-opens,
|
||||||
@@ -174,7 +188,10 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) {
|
|||||||
enabled: isNav && (opts.typeahead ?? true) && !opts.hostsInput,
|
enabled: isNav && (opts.typeahead ?? true) && !opts.hostsInput,
|
||||||
listRef: labelsRef,
|
listRef: labelsRef,
|
||||||
activeIndex,
|
activeIndex,
|
||||||
onMatch: setActiveIndex,
|
onMatch: (i) => {
|
||||||
|
setActiveIndex(i);
|
||||||
|
setHoverIndex(null);
|
||||||
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
const interactions = useInteractions([click, hover, focus, dismiss, role, listNav, typeahead]);
|
const interactions = useInteractions([click, hover, focus, dismiss, role, listNav, typeahead]);
|
||||||
@@ -204,6 +221,8 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) {
|
|||||||
elementsRef,
|
elementsRef,
|
||||||
labelsRef,
|
labelsRef,
|
||||||
activeIndex,
|
activeIndex,
|
||||||
|
highlightIndex: hoverIndex ?? activeIndex,
|
||||||
|
setHoverIndex,
|
||||||
...interactions,
|
...interactions,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
@@ -212,6 +231,8 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) {
|
|||||||
// separately owns the elementsRef/labelsRef registration; this context is the active-state channel.
|
// separately owns the elementsRef/labelsRef registration; this context is the active-state channel.
|
||||||
interface PopoverListContextValue {
|
interface PopoverListContextValue {
|
||||||
activeIndex: number | null;
|
activeIndex: number | null;
|
||||||
|
highlightIndex: number | null;
|
||||||
|
setHoverIndex: (index: number | null) => void;
|
||||||
getItemProps: AnchoredPopover["getItemProps"];
|
getItemProps: AnchoredPopover["getItemProps"];
|
||||||
}
|
}
|
||||||
const PopoverListContext = createContext<PopoverListContextValue | null>(null);
|
const PopoverListContext = createContext<PopoverListContextValue | null>(null);
|
||||||
@@ -259,7 +280,10 @@ export function PopoverItem({
|
|||||||
const ctx = useContext(PopoverListContext);
|
const ctx = useContext(PopoverListContext);
|
||||||
const textLabel = label ?? (typeof children === "string" ? children : undefined);
|
const textLabel = label ?? (typeof children === "string" ? children : undefined);
|
||||||
const { ref, index } = useListItem({ label: textLabel });
|
const { ref, index } = useListItem({ label: textLabel });
|
||||||
const active = ctx?.activeIndex === index;
|
// Roving tabindex / DOM focus follow the KEYBOARD position (activeIndex); the visible highlight
|
||||||
|
// follows the pointer when hovering, else the keyboard (highlightIndex = hoverIndex ?? activeIndex).
|
||||||
|
const focusActive = ctx?.activeIndex === index;
|
||||||
|
const visualActive = ctx?.highlightIndex === index;
|
||||||
|
|
||||||
const ariaProps =
|
const ariaProps =
|
||||||
role === "menuitemradio" || role === "menuitemcheckbox"
|
role === "menuitemradio" || role === "menuitemcheckbox"
|
||||||
@@ -276,8 +300,16 @@ export function PopoverItem({
|
|||||||
rest.onClick?.(e);
|
rest.onClick?.(e);
|
||||||
if (!disabled) onSelect?.();
|
if (!disabled) onSelect?.();
|
||||||
};
|
};
|
||||||
|
// Move the single visual highlight to the hovered item via hoverIndex — NOT floating-ui's activeIndex,
|
||||||
|
// which would drag DOM focus onto the item and blur a menu's free-text input (the create-tag field)
|
||||||
|
// mid-typing. This is what makes one highlight follow the pointer while the input keeps its caret.
|
||||||
|
// (The guard keeps it a no-op when the pointer hasn't crossed into a new row.)
|
||||||
|
const mergedMouseMove = (e: React.MouseEvent<HTMLButtonElement>) => {
|
||||||
|
rest.onMouseMove?.(e);
|
||||||
|
if (ctx && !disabled && index != null && ctx.highlightIndex !== index) ctx.setHoverIndex(index);
|
||||||
|
};
|
||||||
const handlers = ctx
|
const handlers = ctx
|
||||||
? ctx.getItemProps({ ...rest, onClick: mergedClick })
|
? ctx.getItemProps({ ...rest, onClick: mergedClick, onMouseMove: mergedMouseMove })
|
||||||
: { ...rest, onClick: mergedClick };
|
: { ...rest, onClick: mergedClick };
|
||||||
|
|
||||||
return (
|
return (
|
||||||
@@ -288,8 +320,8 @@ export function PopoverItem({
|
|||||||
{...handlers}
|
{...handlers}
|
||||||
// These win over anything in `handlers` — the roving tabindex must not be a caller override,
|
// These win over anything in `handlers` — the roving tabindex must not be a caller override,
|
||||||
// and disabled/className/data-active are the platform's to set.
|
// and disabled/className/data-active are the platform's to set.
|
||||||
tabIndex={active ? 0 : -1}
|
tabIndex={focusActive ? 0 : -1}
|
||||||
data-active={active ? "true" : undefined}
|
data-active={visualActive ? "true" : undefined}
|
||||||
disabled={disabled}
|
disabled={disabled}
|
||||||
className={`${ACTIVE_HIGHLIGHT}${className ? ` ${className}` : ""}`}
|
className={`${ACTIVE_HIGHLIGHT}${className ? ` ${className}` : ""}`}
|
||||||
>
|
>
|
||||||
@@ -349,6 +381,8 @@ function PopoverSurfaceInner({
|
|||||||
isNav,
|
isNav,
|
||||||
selectedIndex,
|
selectedIndex,
|
||||||
activeIndex,
|
activeIndex,
|
||||||
|
highlightIndex,
|
||||||
|
setHoverIndex,
|
||||||
elementsRef,
|
elementsRef,
|
||||||
labelsRef,
|
labelsRef,
|
||||||
} = popover;
|
} = popover;
|
||||||
@@ -402,7 +436,12 @@ function PopoverSurfaceInner({
|
|||||||
positioned element is essential: the popIn keyframe animates `transform`, which would
|
positioned element is essential: the popIn keyframe animates `transform`, which would
|
||||||
otherwise override Floating UI's `transform: translate(x,y)` and park the popover at (0,0)
|
otherwise override Floating UI's `transform: translate(x,y)` and park the popover at (0,0)
|
||||||
for the animation's duration — the "flash in the top-left corner" bug. */}
|
for the animation's duration — the "flash in the top-left corner" bug. */}
|
||||||
<div className={className} style={style}>
|
<div
|
||||||
|
className={className}
|
||||||
|
style={style}
|
||||||
|
// Pointer left the menu → drop the hover highlight so it returns to the keyboard position.
|
||||||
|
onMouseLeave={() => setHoverIndex(null)}
|
||||||
|
>
|
||||||
{children}
|
{children}
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
@@ -411,7 +450,9 @@ function PopoverSurfaceInner({
|
|||||||
// Menu/listbox surfaces host the item registry (FloatingList) + active-state context so nested
|
// Menu/listbox surfaces host the item registry (FloatingList) + active-state context so nested
|
||||||
// <PopoverItem>s can self-register and rove.
|
// <PopoverItem>s can self-register and rove.
|
||||||
const positioned = isNav ? (
|
const positioned = isNav ? (
|
||||||
<PopoverListContext.Provider value={{ activeIndex, getItemProps }}>
|
<PopoverListContext.Provider
|
||||||
|
value={{ activeIndex, highlightIndex, setHoverIndex, getItemProps }}
|
||||||
|
>
|
||||||
<FloatingList elementsRef={elementsRef} labelsRef={labelsRef}>
|
<FloatingList elementsRef={elementsRef} labelsRef={labelsRef}>
|
||||||
{surface}
|
{surface}
|
||||||
</FloatingList>
|
</FloatingList>
|
||||||
|
|||||||
@@ -75,7 +75,7 @@ function TagEditorMenu({
|
|||||||
checked={on}
|
checked={on}
|
||||||
onSelect={() => onToggle(tg.id)}
|
onSelect={() => onToggle(tg.id)}
|
||||||
label={tg.name}
|
label={tg.name}
|
||||||
className={`w-full flex items-center gap-2 text-left text-xs px-2 py-1.5 rounded-md outline-hidden transition hover:bg-card ${
|
className={`w-full flex items-center gap-2 text-left text-xs px-2 py-1.5 rounded-md outline-hidden transition ${
|
||||||
on ? "text-fg" : "text-muted"
|
on ? "text-fg" : "text-muted"
|
||||||
}`}
|
}`}
|
||||||
>
|
>
|
||||||
|
|||||||
@@ -78,7 +78,7 @@ export default function LanguageSwitcher({
|
|||||||
setOpen(false);
|
setOpen(false);
|
||||||
}}
|
}}
|
||||||
label={l.label}
|
label={l.label}
|
||||||
className="w-full flex items-center justify-between gap-2 text-sm px-2 py-2 rounded-lg text-muted outline-hidden transition hover:text-fg hover:bg-card"
|
className="w-full flex items-center justify-between gap-2 text-sm px-2 py-2 rounded-lg text-muted outline-hidden transition hover:text-fg"
|
||||||
>
|
>
|
||||||
<span>{l.label}</span>
|
<span>{l.label}</span>
|
||||||
{l.code === value && <Check className="w-4 h-4 text-accent" />}
|
{l.code === value && <Check className="w-4 h-4 text-accent" />}
|
||||||
|
|||||||
@@ -95,7 +95,7 @@ export default function ViewSwitcher<T extends string>({
|
|||||||
}}
|
}}
|
||||||
label={o.label}
|
label={o.label}
|
||||||
className={`w-full flex items-center gap-2 px-2 py-1.5 rounded-lg text-sm text-left outline-hidden transition ${
|
className={`w-full flex items-center gap-2 px-2 py-1.5 rounded-lg text-sm text-left outline-hidden transition ${
|
||||||
on ? "text-accent" : "text-fg hover:bg-card"
|
on ? "text-accent" : "text-fg"
|
||||||
}`}
|
}`}
|
||||||
>
|
>
|
||||||
<Icon className="w-4 h-4 shrink-0" />
|
<Icon className="w-4 h-4 shrink-0" />
|
||||||
|
|||||||
@@ -198,7 +198,7 @@ function FilterFunnel<T>({
|
|||||||
}
|
}
|
||||||
label={o.label}
|
label={o.label}
|
||||||
className={`flex w-full items-center gap-2 text-left text-xs px-2 py-1.5 rounded-md outline-hidden transition ${
|
className={`flex w-full items-center gap-2 text-left text-xs px-2 py-1.5 rounded-md outline-hidden transition ${
|
||||||
on ? "text-fg" : "text-muted hover:bg-card"
|
on ? "text-fg" : "text-muted"
|
||||||
}`}
|
}`}
|
||||||
>
|
>
|
||||||
<span
|
<span
|
||||||
|
|||||||
Reference in New Issue
Block a user