-
Notifications
You must be signed in to change notification settings - Fork 101
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
chore: add 'Skip to main content' button to the main page #1837
base: master
Are you sure you want to change the base?
Conversation
|
7ba47f3
to
ec9fd33
Compare
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.
While it works good on the main page, it breaks on other pages. I think the good solution is to focus on the content container, it does not have to be an interactive element.
This is done by e.g. wikipedia.com or github.com
ec9fd33
to
a7e2315
Compare
I fixed the above issues:
|
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.
still not perfect, I could not trigger it when the side nav is open. Also AFAIK it should work for ALL pages, the main purpose of this feature is that if I send you a link like https://instructure.design/pr-preview/pr-1837/#CHANGELOG and you are a keyboard only SR user you dont have to tab trough our whole menu. Also its not nice to have a button that does nothing in this case. Cant you just move the focus to the content container?
a7e2315
to
7f7122f
Compare
@matyasf I addressed this issue. Now the 'Skip to main button' should work on every page and also when the nav bar is open. Also added focus to Search bar inside of the navigation when open. Please check the revised test plan. |
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.
nice work, works perfectly now :)
PS: The preview link is not up to date, I guess because of the merge conflict?
Closes: INSTUI-4240 chore: add 'Skip to main content' button to the main page
7f7122f
to
87a856f
Compare
@matyasf I resolved the merge conflict, the preview link seems up-to-date to me now. |
Closes: INSTUI-4240
ISUUE: main page does not have a a 'Skip to main content' button to avoid going through repetitive content. This current implementation is based on this recommendation: https://www.a11y-collective.com/blog/skip-to-main-content/
TEST PLAN:
test skipt to main button:
go through the different types of docs page with screenreader with Tab
1. main page
2. document like /#Alert or /#focus-management
3. changelog page /#CHANGELOG
4. theme page like /#canvas-high-contrast
5. icons page: /#icons
6. error page /#iconsss
the first focusable element on each page should be a 'Skip to main content' button
the button should not be visible when not focused
when the button is clicked, it should bring the focus to the main content
the 'Skip to main content' button also should work when the side navigation is open
test skip to main button focus after page load:
test clicking the hamburger Icon: