From b84db50b699f900faa1ebf8887f43701dc20ed58 Mon Sep 17 00:00:00 2001 From: munzzyy Date: Sat, 4 Jul 2026 21:41:07 -0500 Subject: [PATCH 1/2] fix(IconButton): respect consumer-supplied aria-label aria-label was set after the {...props} spread, so it always overrode a consumer-supplied aria-label with the raw icon token name (e.g. 'trash', 'sparkle'). Screen readers announced the icon's internal name instead of the button's actual purpose. Now a consumer aria-label wins, falling back to the icon name only when none is given. Added tests covering both paths. --- .changeset/fix-iconbutton-aria-label-override.md | 5 +++++ src/components/IconButton/IconButton.test.tsx | 15 +++++++++++++++ src/components/IconButton/IconButton.tsx | 2 +- 3 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 .changeset/fix-iconbutton-aria-label-override.md diff --git a/.changeset/fix-iconbutton-aria-label-override.md b/.changeset/fix-iconbutton-aria-label-override.md new file mode 100644 index 000000000..63f912cb9 --- /dev/null +++ b/.changeset/fix-iconbutton-aria-label-override.md @@ -0,0 +1,5 @@ +--- +'@clickhouse/click-ui': patch +--- + +Fix IconButton silently overriding a consumer-supplied aria-label with the icon name. A consumer-provided aria-label now wins, falling back to the icon name only when none is given. diff --git a/src/components/IconButton/IconButton.test.tsx b/src/components/IconButton/IconButton.test.tsx index 8486ae67d..a8ba8bada 100644 --- a/src/components/IconButton/IconButton.test.tsx +++ b/src/components/IconButton/IconButton.test.tsx @@ -39,6 +39,21 @@ describe('Button', () => { expect(button).toBeDisabled(); }); + describe('accessible name', () => { + it('should fall back to the icon name when no aria-label is given', () => { + const { getByRole } = renderButton({ icon: 'trash' }); + expect(getByRole('button')).toHaveAttribute('aria-label', 'trash'); + }); + + it('should use a consumer-supplied aria-label instead of the icon name', () => { + const { getByRole } = renderButton({ + icon: 'trash', + 'aria-label': 'Delete item', + }); + expect(getByRole('button')).toHaveAttribute('aria-label', 'Delete item'); + }); + }); + describe('Button HTML types', () => { it('should not default to any type when consumer does not specify it, thus behaving as type=submit', () => { const handleSubmit = vi.fn(); diff --git a/src/components/IconButton/IconButton.tsx b/src/components/IconButton/IconButton.tsx index c697422c8..7ec1543d1 100644 --- a/src/components/IconButton/IconButton.tsx +++ b/src/components/IconButton/IconButton.tsx @@ -37,7 +37,7 @@ export const IconButton = forwardRef( disabled={disabled} ref={ref} role="button" - aria-label={iconName} + aria-label={props['aria-label'] ?? iconName} > Date: Sat, 4 Jul 2026 22:12:03 -0500 Subject: [PATCH 2/2] fix(IconButton): hide inner icon from a11y tree, fix empty aria-label fallback The Icon rendered inside IconButton still exposed its own role="img" aria-label, so assistive tech announced two names for one control. Mark it aria-hidden, matching how Button and ButtonGroup already hide their icons. Also switch the aria-label fallback from ?? to ||, so an explicit empty aria-label="" falls back to the icon name instead of leaving the button nameless. --- .../fix-iconbutton-aria-label-override.md | 2 +- src/components/IconButton/IconButton.test.tsx | 17 +++++++++++++++++ src/components/IconButton/IconButton.tsx | 3 ++- 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/.changeset/fix-iconbutton-aria-label-override.md b/.changeset/fix-iconbutton-aria-label-override.md index 63f912cb9..e76669e51 100644 --- a/.changeset/fix-iconbutton-aria-label-override.md +++ b/.changeset/fix-iconbutton-aria-label-override.md @@ -2,4 +2,4 @@ '@clickhouse/click-ui': patch --- -Fix IconButton silently overriding a consumer-supplied aria-label with the icon name. A consumer-provided aria-label now wins, falling back to the icon name only when none is given. +Fix IconButton silently overriding a consumer-supplied aria-label with the icon name. A consumer-provided aria-label now wins, falling back to the icon name when none is given (including an empty string). The inner icon is also marked aria-hidden so the button exposes a single accessible name instead of two. diff --git a/src/components/IconButton/IconButton.test.tsx b/src/components/IconButton/IconButton.test.tsx index a8ba8bada..5884a53e2 100644 --- a/src/components/IconButton/IconButton.test.tsx +++ b/src/components/IconButton/IconButton.test.tsx @@ -52,6 +52,23 @@ describe('Button', () => { }); expect(getByRole('button')).toHaveAttribute('aria-label', 'Delete item'); }); + + it('should fall back to the icon name when aria-label is an empty string', () => { + const { getByRole } = renderButton({ + icon: 'trash', + 'aria-label': '', + }); + expect(getByRole('button')).toHaveAttribute('aria-label', 'trash'); + }); + + it('should hide the inner icon from assistive tech so the button exposes a single accessible name', () => { + const { getByRole, queryByRole } = renderButton({ + icon: 'trash', + 'aria-label': 'Delete item', + }); + expect(getByRole('button')).toHaveAttribute('aria-label', 'Delete item'); + expect(queryByRole('img')).not.toBeInTheDocument(); + }); }); describe('Button HTML types', () => { diff --git a/src/components/IconButton/IconButton.tsx b/src/components/IconButton/IconButton.tsx index 7ec1543d1..be08837a9 100644 --- a/src/components/IconButton/IconButton.tsx +++ b/src/components/IconButton/IconButton.tsx @@ -37,10 +37,11 @@ export const IconButton = forwardRef( disabled={disabled} ref={ref} role="button" - aria-label={props['aria-label'] ?? iconName} + aria-label={props['aria-label'] || iconName} >