Skip to content
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

A11y issue - focus should move to the onboarding card when start() is called #88

Closed
chriskulwik opened this issue Nov 2, 2023 · 4 comments · Fixed by #92
Closed

A11y issue - focus should move to the onboarding card when start() is called #88

chriskulwik opened this issue Nov 2, 2023 · 4 comments · Fixed by #92
Labels
bug Something isn't working released

Comments

@chriskulwik
Copy link

Describe the bug
This is an accessibility issue.

issue 1:
For a user who is not able to use a mouse, they navigate around the page using the tab key to focus on different elements. This doesn't work well. This also happens when clicking next or back.

issue 2:
Another note, focus should be trapped within the onboarding item once focus is established in there.

To Reproduce
issue 1:
Once start() is called, focus should be moved to inside the onboarding item, this is not the case. Currently, start() is called, but the focus is not moved, making it so this user has to hit tab many times, first focusing on every element on the page before focus is transferred to the onboarding item.

issue 2:
Say for example, if I have a close button, back button, and a next button:

  • say we have focus on close
  • hit tab
  • now focus is on back
  • hit tab
  • now focus is on next,
  • hit tab
  • focus goes to the rest of the web page

Expected behavior

issue 1:
Focus should be transferred to the onboarding item when next, back, or start() is called.

issue 2:
Focus should be trapped in the onboarding item

  • say we have focus on close
  • hit tab
  • now focus is on back
  • hit tab
  • now focus is on next,
  • hit tab
  • focus should go back to close

Screenshots
If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

  • OS: mac
  • Browser: chrome
  • Version: package version 2.6.0

Smartphone (please complete the following information):

  • Device: [e.g. iPhone6]
  • OS: [e.g. iOS8.1]
  • Browser [e.g. stock browser, safari]
  • Version [e.g. 22]

Additional context
Add any other context about the problem here.

@chriskulwik chriskulwik added the bug Something isn't working label Nov 2, 2023
@rozsazoltan
Copy link
Contributor

Wow, that's a completely valid observation! A real issue that needs to be addressed.

@chriskulwik
Copy link
Author

I was able to get around this.

Issue 1 workaround:

add this to every step

on: {
      beforeStep: () => onBeforeStep(),
      afterStep: () => onAfterStep(),
    },

onBeforeStep and onAfterStep are defined as such:

const onBeforeStep = () => {
  document.addEventListener('keydown', keydownHandler);
};

const onAfterStep = () => {
  document.removeEventListener('keydown', keydownHandler);
};

keydownHandler is defined as such:

const keydownHandler = (event: KeyboardEvent) => {
  if (event.key === 'Tab') {
    /*  If you hit tab, the close button will be focused instead of the primary. This is bc the keydown event is
     *  processed first (primary btn is focused), then the browser normally handles the tab press, thus ending on
     *  the close button. */
    const primaryBtn = document.querySelector('button.v-onboarding-btn-primary');
    // @ts-ignore this is ok, focus on primary button if it exists
    primaryBtn?.focus();
    document.removeEventListener('keydown', keydownHandler);
  }
};

Issue 2 workaround:

wrap VOnboardingWrapper in a the FocusLoop component from vue-a11y

<FocusLoop>
   <VOnboardingWrapper ref="wrapper" :steps="steps" :options="options" />
 </FocusLoop>

and a visual accessibility fix, to see which button has focus:

.v-onboarding-item {

  button:focus {
    outline-color: black;
  }
}

If this changes look good, and you think they'd be compatible with the code base, I'm happy to make a fork and work on integrating these changes.

@fatihsolhan
Copy link
Owner

Hi all! Thank you for you interest and potential suggestions for this issue, I appreciate it. I've implemented a focus-trap feature and it should be live soon

Copy link

github-actions bot commented Feb 9, 2024

🎉 This issue has been resolved in version 2.8.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants