14744 tour not coded to best practice

[nagolucky18]

Prerequisites

• I have added steps to test this contribution in the description below

If there's an existing issue for this PR then this fixes 14744

Description

The following changes were made:

1. The focusLockService function is now passed into the TourStepDirective function in umbtourstep.directive.js.
2. HTML properties for accessibility were added to umb-tour.html, umb-tour-step-content.html, umb-tour-step-counter.html, umb-tour-step-footer.html, umb-tour-step-header.html. These properties include tabindex, role, and aria-label.

Prerequisites to test:

1. A screen reader has been downloaded on the PC being used to test. NVDA was used to test during development.

Steps to test:

1. From the Umbraco home page, open any tour via the right side panel.
2. With the screen reader enabled, press TAB until the dialog is reached. "Tour Dialog" should be announced when the dialog is reached.
3. Continue to press TAB. For each element, the content of the element will be announced by the screen reader.
4. When the last button is reached, press Shift + Tab to navigate backwards through the dialog. All contents should be announced by the screen reader.

Note that for certain screen readers and certain screen reader versions, the behavior may be unexpected due to a bug in the screen reader itself. Certain browser versions may also have bugs that produce unexpected behaviors with screen readers.

No unit tests were added, though I can add a few if necessary.

[github-actions]

Hi there @nagolucky18, thank you for this contribution! 👍

While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:

• It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
• The automated tests all pass (see "Checks" tab on this PR)
• The level of security for this contribution is the same or improved
• The level of performance for this contribution is the same or improved
• Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
• If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
• 💡 The contribution looks original and the contributor is presumably allowed to share it

Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution.

If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

[georgebid]

Hey @nagolucky18, thanks a lot for creating a PR to fix this one, someone from our core collaborators team will review this soon. And congratulations on your first PR! 🥳

[georgebid]

Hey @nagolucky18 I have just been reviewing this one and so far, it looks pretty good to me, much easier to use with a screen reader! The only issue I have spotted is when the tour instructs the user to select the buttons behind the screen reader, e.g this one:

Previously, you could still tab to your user profile (even if it took forever!), but with your changes its no longer possible to do this without ending the tour, or using your mouse. What are your thoughts on this? It might be a discussion point for the original issue. Let me know what you think! 😄

[nagolucky18]

@georgebid Thanks for catching this. The current implementation makes resolving this tricky. Currently the HTML "inert" property is added to the mainwrapper div, however this makes all children of mainwrapper inert. This cannot be overriden. To that end, "aria-hidden" would also not work since that too cannot be overridden.

Selecting only specific nodes to apply "inert" to would work, but might be difficult to do. I can take a look at implementing something like that.

[georgebid]

Sorry for the delay @nagolucky18, but yes, I didn't think it would be the easiest fix, I think I will double-check this with HQ or maybe the accessibility team to see if going with your solution would be the best option, as I do think it's an improvement compared to the current. I will get back to you soon 😄

[georgebid]

Hey @nagolucky18, once again, thank you for your patience! I've spoken internally regarding this PR, and we are happy that your fix is definitely an improvement, regardless of what I had flagged. Therefore, I will merge this one in 😸 Thanks again for fixing #14744, you should see this in the next release.

I've also just noticed that this was originally your first Umbraco PR, congrats! I know you have had other PRs merged in since, but I am not sure if you have been awarded the contributor badge yet? 🥳If not, please provide us with your account name on the forum, we will gladly assign our shiny contributor's badge to you 🏅