Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STSMACOM-882 <NoteFields> improve "Display as pop-up" markup and fix label a11y issue. #1553

Merged
merged 3 commits into from
Jan 8, 2025

Conversation

BogdanDenis
Copy link
Contributor

Description

axe previously showed an a11y issue related to checkbox group labels. Latest code doesn't show this issue anymore, but applied the fix anyways.
Re-organized "Display as pop-up" checkbox group markup.

Screenshots

image

Issues

STSMACOM-882

@BogdanDenis BogdanDenis requested review from Dmytro-Melnyshyn and a team January 6, 2025 13:28
Copy link

github-actions bot commented Jan 6, 2025

Bigtest Unit Test Results

  1 files  ±0    1 suites  ±0   14s ⏱️ ±0s
509 tests ±0  467 ✅ ±0  42 💤 ±0  0 ❌ ±0 
511 runs  ±0  469 ✅ ±0  42 💤 ±0  0 ❌ ±0 

Results for commit 6fadbe2. ± Comparison against base commit 18fdf67.

♻️ This comment has been updated with latest results.

Comment on lines 93 to 97
<Label for="display-as-popup-group">
<span className={styles.heading}>
<FormattedMessage id="stripes-smart-components.notes.displayAsPopup.label" />
</span>
</Label>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the 2nd approach from the ticket is what we need, it will announce "grouping" and be an indicator for the association of checkboxes. This can be seen with NVDA, axe dev tools doesn't show it. The for attribute shouldn't work in React for label tags.

@Dmytro-Melnyshyn Dmytro-Melnyshyn requested a review from a team January 6, 2025 15:11
@Dmytro-Melnyshyn Dmytro-Melnyshyn requested a review from a team January 7, 2025 10:22
@BogdanDenis BogdanDenis requested a review from a team as a code owner January 8, 2025 12:01
Copy link

sonarqubecloud bot commented Jan 8, 2025

@BogdanDenis BogdanDenis merged commit 101386a into master Jan 8, 2025
15 checks passed
@BogdanDenis BogdanDenis deleted the STSMACOM-882 branch January 8, 2025 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants