-
-
Notifications
You must be signed in to change notification settings - Fork 156
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
Accessibility: Setting headings' visual styles in an accessible way #320
Accessibility: Setting headings' visual styles in an accessible way #320
Conversation
@obulat I think there is some problem with testing. I ran the "npm run test" command and got some errors even initially without making any changes. |
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.
Thank you for your great work, @neeraj-2! This PR can be merge after a minor style change is added:
The letter spacing style should be moved from 'Stepper.vue' to 'StepHeader.vue' so that the letters in the headings are not spaced out too wide:
chooser/src/components/Stepper.vue
Lines 309 to 311 in a127966
.step-header__title.b-header { | |
letter-spacing: normal; | |
} |
Actually, this was my mistake: this logic should be tested in integration tests, not the unit tests as it is now. Can you please delete the failing test? |
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.
Great work! Thank you.
This test will be later added to the integration test suite.
Thanks, @zackkrida, and @obulat |
Fixes
Fixes #319 by @obulat
Description
It sets the headings' visual styles in an accessible way and thus enhances accessibility.
Screenshots
Checklist
Update index.md
).main
ormaster
).visible errors.
Developer Certificate of Origin
Developer Certificate of Origin