fix(feed): open the view menu on the active item, group its radio items
Seeding the roving index from an effect only scheduled the update, so the focus effect still read the previous index and focused the wrong item for one render — a screen reader announced both. Seed it where the menu opens instead. The modes are one exclusive choice, so they also need a group to scope their checked-ness to.
This commit is contained in:
@@ -41,14 +41,19 @@ export default function ViewSwitcher<T extends string>({
|
||||
// just clicked. Escape is handled on the menu itself (below), where returning focus is right.
|
||||
useDismiss(open, () => close({ refocus: false }), [btnRef, menuRef]);
|
||||
|
||||
// A `value` outside `options` is a caller bug; degrade to the first option rather than render
|
||||
// nothing. The menu then shows no checked item, which is honest — nothing IS selected.
|
||||
const currentIdx = Math.max(
|
||||
0,
|
||||
options.findIndex((o) => o.id === value)
|
||||
);
|
||||
// Open on the active item, the way a menu is expected to.
|
||||
useEffect(() => {
|
||||
if (open) setFocusIdx(currentIdx);
|
||||
}, [open, currentIdx]);
|
||||
// Seeding the roving index HERE — rather than in an effect keyed on `open` — matters: an effect
|
||||
// would only schedule the update, so the focus effect below would still see the previous index
|
||||
// and focus the wrong item for one render before correcting.
|
||||
function openMenu() {
|
||||
setFocusIdx(currentIdx);
|
||||
setOpen(true);
|
||||
}
|
||||
useEffect(() => {
|
||||
if (open) itemRefs.current[focusIdx]?.focus();
|
||||
}, [open, focusIdx]);
|
||||
@@ -85,7 +90,7 @@ export default function ViewSwitcher<T extends string>({
|
||||
<div className="relative">
|
||||
<button
|
||||
ref={btnRef}
|
||||
onClick={() => (open ? close() : setOpen(true))}
|
||||
onClick={() => (open ? close() : openMenu())}
|
||||
aria-haspopup="menu"
|
||||
aria-expanded={open}
|
||||
aria-label={`${label}: ${current.label}`}
|
||||
@@ -103,32 +108,36 @@ export default function ViewSwitcher<T extends string>({
|
||||
onKeyDown={onMenuKeyDown}
|
||||
className="glass-menu absolute right-0 top-full mt-1 z-30 min-w-44 p-1.5 rounded-xl animate-[popIn_0.16s_ease]"
|
||||
>
|
||||
{options.map((o, i) => {
|
||||
const Icon = o.icon;
|
||||
const on = o.id === value;
|
||||
return (
|
||||
<button
|
||||
key={o.id}
|
||||
ref={(el) => {
|
||||
itemRefs.current[i] = el;
|
||||
}}
|
||||
role="menuitemradio"
|
||||
aria-checked={on}
|
||||
tabIndex={i === focusIdx ? 0 : -1}
|
||||
onClick={() => {
|
||||
onChange(o.id);
|
||||
close();
|
||||
}}
|
||||
className={`w-full flex items-center gap-2 px-2 py-1.5 rounded-lg text-sm text-left transition ${
|
||||
on ? "text-accent" : "text-fg hover:bg-card"
|
||||
}`}
|
||||
>
|
||||
<Icon className="w-4 h-4 shrink-0" />
|
||||
<span className="flex-1 truncate">{o.label}</span>
|
||||
{on && <Check className="w-3.5 h-3.5 shrink-0" />}
|
||||
</button>
|
||||
);
|
||||
})}
|
||||
{/* The modes are one exclusive choice, so the radio items need a group to scope their
|
||||
checked-ness to — otherwise they read as independent checkable items. */}
|
||||
<div role="group">
|
||||
{options.map((o, i) => {
|
||||
const Icon = o.icon;
|
||||
const on = o.id === value;
|
||||
return (
|
||||
<button
|
||||
key={o.id}
|
||||
ref={(el) => {
|
||||
itemRefs.current[i] = el;
|
||||
}}
|
||||
role="menuitemradio"
|
||||
aria-checked={on}
|
||||
tabIndex={i === focusIdx ? 0 : -1}
|
||||
onClick={() => {
|
||||
onChange(o.id);
|
||||
close();
|
||||
}}
|
||||
className={`w-full flex items-center gap-2 px-2 py-1.5 rounded-lg text-sm text-left transition ${
|
||||
on ? "text-accent" : "text-fg hover:bg-card"
|
||||
}`}
|
||||
>
|
||||
<Icon className="w-4 h-4 shrink-0" />
|
||||
<span className="flex-1 truncate">{o.label}</span>
|
||||
{on && <Check className="w-3.5 h-3.5 shrink-0" />}
|
||||
</button>
|
||||
);
|
||||
})}
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user