-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
fix: a11y modal heading issue #17447
fix: a11y modal heading issue #17447
Conversation
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for v11-carbon-react ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17447 +/- ##
=======================================
Coverage 77.07% 77.07%
=======================================
Files 409 409
Lines 14021 14021
Branches 4355 4307 -48
=======================================
Hits 10807 10807
- Misses 3043 3044 +1
+ Partials 171 170 -1 ☔ View full report in Codecov by Sentry. |
I'm wondering if this is actually required since the current component structure looks correct. There is no content following the first |
I had the same concerns. Raghu told the product team to have this heirarchy in modal. Check here carbon-design-system/ibm-products#6019 . So raised the PR , not sure its required or not. |
Ah ok. I'll reach out to Ragu to discuss during the next QA meeeting. |
@riddhybansal whats the status on this one? |
Heya @alisonjoseph I think @kennylam had a word with Raghu and this fix is required , so I think you can review it !! @kennylam can you confirm this ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, sorry for forgetting to update. Yes, spoke to Ragu about this and he said that either way is not really an issue.
eeaebc9
Closes #17397
Maintain hierarchical order for the Heading levels :
Recommended to present the Modal Main heading in H2.
Changelog
Changed
Testing / Reviewing
Modal and composed modal heading should be in h2