feat(SkipToContent): Add SkipToContent to demos

[rebeccaalpert]

As part of this, I found a bug with the PatternFly page component: patternfly/patternfly-react#11162. I am skipping a skip to content for now, but adding one for the chatbot. I was able to get this working with refs.

I don't know if we want to call this out more explicitly in docs @edonehoo - let me know. We may want to add a11y guidelines like https://www.canaxess.com.au/infocard/chatbots/ or https://mitre.github.io/chatbot-accessibility-playbook/docs/4_4.html.

@thatblindgeye - Would you want to handle the skip to content link differently from what I'm doing here? I have it focusing the toggle on the chatbot-container example and the message box on the embedded example. This does mean that when you're in docked or fullscreen that it makes less sense - should we focus some other part of the chatbot in this case? Let me know. I am leaving it off of the attachment demos since Nicole wanted them to be simpler.

Demos are here:

• https://virtual-assistant-pr-virtual-assistant-296.surge.sh/patternfly-ai/chatbot/chatbot-container/react-demos/embedded-chatbot/
• https://virtual-assistant-pr-virtual-assistant-296.surge.sh/patternfly-ai/chatbot/chatbot-container/react-demos/basic-chatbot/

[patternfly-build]

Preview: https://virtual-assistant-pr-virtual-assistant-296.surge.sh

A11y report: https://virtual-assistant-pr-virtual-assistant-296-a11y.surge.sh

[thatblindgeye]

@rebeccaalpert for the embedded example, it might be better to place focus on the outermost chatbot container. Putting focus on the message log can work, but I'm thinking it could be confusing to start there as it may not be known there's the additional contents in the chatbot header (menu and version select). One caveat to that is that placing focus on that outermost container would result in a lot of content being announced. Could potentially resolve that by adding a role="region" and an aria-label to the wrapper, though.

For the example with toggle, the default case of focusing the button looks good. For the docked or fullscreen options, it'd be great to be able to have behavior similar to the above, where maybe the outermost chatbot wrapper gets focus instead of the button.

[thatblindgeye]

This is looking great 😍 Just a few more comments

• As you mentioned on Slack, we could use a <section> element instead of the div role region. It would follow the whole "use native HTML over aria when possible" recommendation so we could make that update in this PR.
• For the toggle button example (with "overlay" selected), I'm wondering if we should also shift the focus based on whether the chatbot overlay is open or not: if closed focus the toggle button, if open focus the chatbot wrapper like in the other instances

[edonehoo]

I started reviewing and then surge crashed for me 😅 but going ahead and sharing what I started. I might be misunderstanding what this feature is, please correct me if so!

[thatblindgeye]

Overall this is looking good. One thing that would be worth opening a followup issue for would be to try and resolve an issue with a focus indicator after clicking the skip link. Right now it doesn't seem like there's any outline on an element to indicate what has focus after clicking it via keyboard only (except for the example where the "docked" option is selected, so it's not a consistent issue).

One solution would be to see if there's some styling interfering with the native browser focus outline or tweak a bit with what wrapper container is receiving focus if possible. It does seem like it might sorta be a styling issue; at least, when I fiddle with the height of the embedded chatbot to make it 200px in height, I can see th native browser focus outline, but it's getting cut off/hidden:

Another solution would be to move focus to the first focusable item in the chatbot (which in these demos would either be the toggle to open the chatbot, or the menu button inside the chatbot) and maybe updating the aria labeling to something more unique ("Chatbot message history"? "Chatbot history"? etc). At least with VoiceOver, when focus is set to that menu button, VO does announce the button + its label, as well as the "Chatbot" region, but I haven't tested in anything else to see if that behavior is consistent across other AT so would be worth trying to test that out a little more before attempting this one.

Either way, not a blocker for this PR if there's a goal to get this in sooner than later, and not totally sure how deep a rabbit hole fiddling with styling and such will be.

[edonehoo]

@rebeccaalpert okay I updated my previous comments now that I'm looking at the right button! lmk if those changes make sense. I do think it may be worth also adding a specific example for this button, just because it's a little bit buried in the demo?

[rebeccaalpert]

Right now it doesn't seem like there's any outline on an element to indicate what has focus after clicking it via keyboard only (except for the example where the "docked" option is selected, so it's not a consistent issue).

That's a good callout. We'd have to refactor some things I think - I'll open a new issue for the focus indicator here #305.

[rebeccaalpert]

@edonehoo - Just added a direct example; let me know your thoughts!

[rebeccaalpert]

@thatblindgeye - I had an idea re: the focus ring. I managed to get it working for overlay (already working for docked, like you said). Fullscreen will be a problem since it's at 100% of the screen and embedded because the PatternFly page has overflow: hidden. I moved the focus in this case to the button as suggested. Does this work for you? We would need to leave the configuration up to the consumer since they can drop the header or put whatever they want in the header. I can add something about this to the docs.

[thatblindgeye]

@rebeccaalpert I think what you have currently in the PR works. It does make sense to just focus the first focusable thing in the chatbot container if it's taking up the entire window/page -- if the intent is that it's encompasses the entire width/height of the page then likely any outline won't be easily perceivable. For the other options I think it looks good as well. If this is all really more showing "here's how you can go about doing this" rather than "here's the behavior you'll have out of the box", I think what you have makes sense (+ some verbiage maybe as you said). We can always revisit and tweak our recommendation if we find/think of anything new.

[edonehoo]

Looks great! Very minor suggestion, just to break up the paragraph for easier scanning

[github-actions]

🎉 This PR is included in version 2.1.0-prerelease.12 🎉

The release is available on:

• GitHub release
• npm package (@prerelease dist-tag)

Your semantic-release bot 📦🚀