Fix M15: replace role=button divs with native button elements in LibraryView
Both interactive headers in LibraryView used role="button" on divs, which is invalid when they contain child interactive elements (group header had nested All/None and Download buttons). - Group header (groupHead): split into an outer flex div + a native <button> (groupToggle style) for the toggle/expand action + sibling Fluent Buttons for All/None and Download. The toggle button gets aria-expanded and a composite aria-label (title + item count). e.stopPropagation() removed from the sibling buttons since they are no longer inside the toggle. - Source card header (cardHead): converted from a role="button" div to a native <button> with width:100%, border:none, and background:transparent. aria-label set to source.title. tabIndex/onKeyDown removed (native button handles keyboard by default). typecheck + 242 tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+1
-1
@@ -263,7 +263,7 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
|
|||||||
- [ ] **M14 — Settings search mutates React-owned DOM.** The search toggles each card's
|
- [ ] **M14 — Settings search mutates React-owned DOM.** The search toggles each card's
|
||||||
`el.style.display` directly; fragile (breaks if a card ever gets a conditional `style`) and
|
`el.style.display` directly; fragile (breaks if a card ever gets a conditional `style`) and
|
||||||
matches on `textContent` incl. hidden text. Prefer state-driven filtering.
|
matches on `textContent` incl. hidden text. Prefer state-driven filtering.
|
||||||
- [ ] **M15 — Nested interactive controls in `role="button"` (a11y).** LibraryView's group header
|
- [x] **M15 — Nested interactive controls in `role="button"` (a11y).** LibraryView's group header
|
||||||
is a `role="button"` div containing `<Button>`s (All / Download) — invalid ARIA / keyboard
|
is a `role="button"` div containing `<Button>`s (All / Download) — invalid ARIA / keyboard
|
||||||
semantics. Make the header a real element with sibling buttons.
|
semantics. Make the header a real element with sibling buttons.
|
||||||
- [x] **M16 — No renderer error boundary.** An exception in any view unmounts the whole shell to
|
- [x] **M16 — No renderer error boundary.** An exception in any view unmounts the whole shell to
|
||||||
|
|||||||
@@ -124,6 +124,11 @@ const useStyles = makeStyles({
|
|||||||
alignItems: 'center',
|
alignItems: 'center',
|
||||||
gap: '12px',
|
gap: '12px',
|
||||||
padding: '12px 14px',
|
padding: '12px 14px',
|
||||||
|
width: '100%',
|
||||||
|
border: 'none',
|
||||||
|
backgroundColor: 'transparent',
|
||||||
|
fontFamily: tokens.fontFamilyBase,
|
||||||
|
textAlign: 'left',
|
||||||
cursor: 'pointer',
|
cursor: 'pointer',
|
||||||
':hover': { backgroundColor: tokens.colorNeutralBackground1Hover }
|
':hover': { backgroundColor: tokens.colorNeutralBackground1Hover }
|
||||||
},
|
},
|
||||||
@@ -164,9 +169,22 @@ const useStyles = makeStyles({
|
|||||||
display: 'flex',
|
display: 'flex',
|
||||||
alignItems: 'center',
|
alignItems: 'center',
|
||||||
gap: '8px',
|
gap: '8px',
|
||||||
padding: '6px 4px',
|
padding: '2px 4px'
|
||||||
|
},
|
||||||
|
groupToggle: {
|
||||||
|
display: 'flex',
|
||||||
|
alignItems: 'center',
|
||||||
|
gap: '8px',
|
||||||
|
flexGrow: 1,
|
||||||
|
minWidth: 0,
|
||||||
|
padding: '4px 4px',
|
||||||
|
border: 'none',
|
||||||
|
backgroundColor: 'transparent',
|
||||||
color: tokens.colorNeutralForeground2,
|
color: tokens.colorNeutralForeground2,
|
||||||
cursor: 'pointer',
|
cursor: 'pointer',
|
||||||
|
fontFamily: tokens.fontFamilyBase,
|
||||||
|
fontSize: tokens.fontSizeBase300,
|
||||||
|
textAlign: 'left',
|
||||||
...shorthands.borderRadius(tokens.borderRadiusMedium),
|
...shorthands.borderRadius(tokens.borderRadiusMedium),
|
||||||
':hover': { backgroundColor: tokens.colorNeutralBackground1Hover }
|
':hover': { backgroundColor: tokens.colorNeutralBackground1Hover }
|
||||||
},
|
},
|
||||||
@@ -413,25 +431,23 @@ export function LibraryView(): React.JSX.Element {
|
|||||||
const groupActionable = groupActionableItems.length
|
const groupActionable = groupActionableItems.length
|
||||||
const allOn = groupActionable > 0 && groupActionableItems.every((it) => selected.has(it.id))
|
const allOn = groupActionable > 0 && groupActionableItems.every((it) => selected.has(it.id))
|
||||||
return (
|
return (
|
||||||
<div
|
<div className={styles.groupHead}>
|
||||||
className={mergeClasses(styles.groupHead, focus.focusRing)}
|
<button
|
||||||
onClick={() => toggleGroupExpand(row.title)}
|
type="button"
|
||||||
role="button"
|
className={mergeClasses(styles.groupToggle, focus.focusRing)}
|
||||||
tabIndex={0}
|
onClick={() => toggleGroupExpand(row.title)}
|
||||||
onKeyDown={(e) => (e.key === 'Enter' || e.key === ' ') && toggleGroupExpand(row.title)}
|
aria-expanded={open}
|
||||||
aria-expanded={open}
|
aria-label={`${row.title}, ${row.items.length} items`}
|
||||||
>
|
>
|
||||||
{open ? <ChevronDownRegular /> : <ChevronRightRegular />}
|
{open ? <ChevronDownRegular /> : <ChevronRightRegular />}
|
||||||
<AppsListRegular />
|
<AppsListRegular />
|
||||||
<span className={styles.groupTitle}>{row.title}</span>
|
<span className={styles.groupTitle}>{row.title}</span>
|
||||||
<Caption1 className={styles.srcSub}>{row.items.length}</Caption1>
|
<Caption1 className={styles.srcSub}>{row.items.length}</Caption1>
|
||||||
|
</button>
|
||||||
<Button
|
<Button
|
||||||
size="small"
|
size="small"
|
||||||
appearance="subtle"
|
appearance="subtle"
|
||||||
onClick={(e) => {
|
onClick={() => toggleGroup(row.items, !allOn)}
|
||||||
e.stopPropagation()
|
|
||||||
toggleGroup(row.items, !allOn)
|
|
||||||
}}
|
|
||||||
>
|
>
|
||||||
{allOn ? 'None' : 'All'}
|
{allOn ? 'None' : 'All'}
|
||||||
</Button>
|
</Button>
|
||||||
@@ -440,10 +456,7 @@ export function LibraryView(): React.JSX.Element {
|
|||||||
appearance="subtle"
|
appearance="subtle"
|
||||||
icon={<ArrowDownloadRegular />}
|
icon={<ArrowDownloadRegular />}
|
||||||
disabled={groupActionable === 0}
|
disabled={groupActionable === 0}
|
||||||
onClick={(e) => {
|
onClick={() => downloadGroup(row.items)}
|
||||||
e.stopPropagation()
|
|
||||||
downloadGroup(row.items)
|
|
||||||
}}
|
|
||||||
>
|
>
|
||||||
Download{groupActionable > 0 ? ` ${groupActionable}` : ''}
|
Download{groupActionable > 0 ? ` ${groupActionable}` : ''}
|
||||||
</Button>
|
</Button>
|
||||||
@@ -729,13 +742,12 @@ function SourceCard({
|
|||||||
const focus = useFocusStyles()
|
const focus = useFocusStyles()
|
||||||
return (
|
return (
|
||||||
<div className={styles.card}>
|
<div className={styles.card}>
|
||||||
<div
|
<button
|
||||||
|
type="button"
|
||||||
className={mergeClasses(styles.cardHead, focus.focusRing)}
|
className={mergeClasses(styles.cardHead, focus.focusRing)}
|
||||||
onClick={onToggleExpand}
|
onClick={onToggleExpand}
|
||||||
role="button"
|
|
||||||
tabIndex={0}
|
|
||||||
onKeyDown={(e) => (e.key === 'Enter' || e.key === ' ') && onToggleExpand()}
|
|
||||||
aria-expanded={expanded}
|
aria-expanded={expanded}
|
||||||
|
aria-label={source.title}
|
||||||
>
|
>
|
||||||
{expanded ? <ChevronDownRegular /> : <ChevronRightRegular />}
|
{expanded ? <ChevronDownRegular /> : <ChevronRightRegular />}
|
||||||
<div className={styles.srcIcon}>
|
<div className={styles.srcIcon}>
|
||||||
@@ -755,7 +767,7 @@ function SourceCard({
|
|||||||
{source.channel && source.channel !== source.title ? ` · ${source.channel}` : ''}
|
{source.channel && source.channel !== source.title ? ` · ${source.channel}` : ''}
|
||||||
</Caption1>
|
</Caption1>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</button>
|
||||||
{expanded && children}
|
{expanded && children}
|
||||||
</div>
|
</div>
|
||||||
)
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user