codeant-ai-for-open-source[bot] commented on code in PR #37581:
URL: https://github.com/apache/superset/pull/37581#discussion_r2747383025
##########
superset-frontend/src/components/ListView/ListView.tsx:
##########
@@ -201,31 +202,34 @@ const ViewModeToggle = ({
setMode: (mode: 'table' | 'card') => void;
}) => (
<ViewModeContainer>
- <div
- role="button"
- tabIndex={0}
- onClick={e => {
- e.currentTarget.blur();
- setMode('card');
- }}
- className={cx('toggle-button', { active: mode === 'card' })}
- >
- <Icons.AppstoreOutlined iconSize="xl" />
- </div>
- <div
- role="button"
- tabIndex={0}
- onClick={e => {
- e.currentTarget.blur();
- setMode('table');
- }}
- className={cx('toggle-button', { active: mode === 'table' })}
- >
- <Icons.UnorderedListOutlined iconSize="xl" />
- </div>
+ <Tooltip title={t('Grid view')}>
+ <div
+ role="button"
+ tabIndex={0}
+ onClick={e => {
+ e.currentTarget.blur();
+ setMode('card');
+ }}
+ className={cx('toggle-button', { active: mode === 'card' })}
+ >
+ <Icons.AppstoreOutlined iconSize="xl" />
+ </div>
+ </Tooltip>
+ <Tooltip title={t('List view')}>
+ <div
+ role="button"
+ tabIndex={0}
Review Comment:
**Suggestion:** Toggle buttons do not expose their pressed state to
assistive technologies: the divs use role="button" and visually indicate active
state via className, but they lack an `aria-pressed` attribute. Add
`aria-pressed` tied to the active condition so screen readers can expose the
toggle state. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Screen readers don't announce toggle state.
- ⚠️ Affects ListView view-mode toggles accessibility.
```
</details>
```suggestion
aria-pressed={mode === 'card'}
onClick={e => {
e.currentTarget.blur();
setMode('card');
}}
className={cx('toggle-button', { active: mode === 'card' })}
>
<Icons.AppstoreOutlined iconSize="xl" />
</div>
</Tooltip>
<Tooltip title={t('List view')}>
<div
role="button"
tabIndex={0}
aria-pressed={mode === 'table'}
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the app and load a page that shows the ListView view-mode toggle
(ListView
component).
2. Open an accessibility inspector or screen reader (e.g., VoiceOver/NVDA)
and navigate to
the toggle.
3. Inspect the toggle element at ListView.tsx:205-230: it has role="button"
and visual
active class,
but no aria-pressed attribute is present to convey state to assistive
tech.
4. Observe assistive tech does not announce the pressed/toggled state;
adding aria-pressed
(aria-pressed={mode === 'card' / mode === 'table'}) exposes state to
screen readers.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/components/ListView/ListView.tsx
**Line:** 209:221
**Comment:**
*Possible Bug: Toggle buttons do not expose their pressed state to
assistive technologies: the divs use role="button" and visually indicate active
state via className, but they lack an `aria-pressed` attribute. Add
`aria-pressed` tied to the active condition so screen readers can expose the
toggle state.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
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]
