rebenitez1802 commented on PR #41470:
URL: https://github.com/apache/superset/pull/41470#issuecomment-4906946042
From my agent reviewer:
🔴 High — label renders as visible text, not just an accessible name
Verified against the code: IconButton.tsx:52 renders {label &&
<StyledSpan>{label}</StyledSpan>}, and StyledSpan (IconButton.tsx:38-40) has
only margin-left — no sr-only/visibility/clip (a full-frontend grep for
visually-hidden utilities finds nothing touching this component), and the
HoverMenu container (HoverMenu.tsx:38-49, .hover-menu--left, 24px wide) has no
overflow:hidden. The three sites previously passed no label (icon-only); after
this PR they render “Delete component” / “Row settings” / “Column settings” as
on-screen text next to the icon on hover in edit mode — an unintended visual
change the PR description doesn’t mention. Fix: feed the accessible name
without the visible span — add a distinct ariaLabel prop applied only to
StyledDiv (or visually-hide the span when used purely as a label), giving the
screen-reader name with zero visual change.
🟡 Medium — Control is still not keyboard-operable (WCAG 2.1.1)
IconButton is a <div role="button" tabIndex={0}> with onClick but no
onKeyDown/onKeyPress (IconButton.tsx:42-54), so Enter/Space don’t activate it —
a keyboard/SR user can now hear the name but still can’t press the button, and
the PR’s test plan checks announcement but never activation. Pre-existing and
arguably out of scope, but the cleanest fix for both this and the High finding
is to render a real <button>, which yields name + keyboard operability for free.
🟢 Low — No test coverage despite an existing pattern
No IconButton.test.tsx/DeleteComponentButton.test.tsx, and the only
IconButton-touching tests (Column.test.tsx:180, Row.test.tsx:222) are
test.skip; Divider.test.tsx already asserts
getByRole('button')…toHaveAttribute('aria-label', …). A getByRole('button', {
name: 'Row settings' }) assertion would lock in intent and catch the
visible-span regression — add it after resolving the High finding.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
