| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM! We should also ensure this gets updated in core and documented on the site.
Sorry, something went wrong.
|
PatternFly-React preview: https://patternfly-react-pr-2519.surge.sh |
Sorry, something went wrong.
There was a problem hiding this comment.
Looks good, can you also update the page demos
Sorry, something went wrong.
There was a problem hiding this comment.
This is awesome! Thanks for all your work researching this piece and contributing it back.
Sorry, something went wrong.
There was a problem hiding this comment.
Looks great so far @seanforyou23 do you mind opening a React issue for tracking purposes.
I had one question. If mainContainerId is not set by consumer, does this work as expected? Maybe we need to add in a test for that?
Sorry, something went wrong.
|
@tlabaj good catch, I updated the tests to reflect what we can expect in the current state. If users don't supply a mainContainerId then things will not work as expected. I'd like to make it a required prop at the next available opportunity for breaking changes. |
Sorry, something went wrong.
Sorry, something went wrong.
|
@seanforyou23 let's open a follow up issue to make that required so we can pull that into the next breaking change release. |
Sorry, something went wrong.
|
@seanforyou23 I opened issue #2597 for the breaking change. |
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
|
Your changes have been released in:
Thanks for your contribution! 🎉 |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
What: Currently, skip to content link sends users to an in-page anchor that sits alongside the main page content container. This generally has the desired effect for sighted keyboard users, however, for screen reader users (SRU) this basically leads to a dead end. It's a dead-end in that the target anchor doesn't contain the page content, so nothing further is announced after the element has been focused.
I think what would be better is if we allow the main content container to carry an id, which can be used as the target of the SkipToContent component. Doing this along with supplying a tabindex="-1" for the main content area gives us a solid hook for informing SRU of general route changes and informing them of what content is now available.
For sighted users, I don't think this will cause any difference in behavior (we may want to add a style like outline: none to the primary page container, although I don't recommend this.
Additional issues: #2591
These changes are driven by a technique suggested in https://dequeuniversity.com where in response to dynamic content changes (like navigating to a new "Page" in a SPA) we want to send focus to the new content so that it is announced and SRU receive some feedback about the new content.