-
-
Notifications
You must be signed in to change notification settings - Fork 645
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
Accessibility support #198
Comments
Hi @gorner, thanks for bringing this to our attention. We did not have any specific plans, but a11y is a good goal for all projects these days. If you have a set of things you'd like us to look at and/or wanted to help implement some of those things, we'd be glad to take a look 😃 |
Fair enough, I'm planning to take a look in the next few days/weeks to better articulate what we'd be looking for. |
@gorner any updates on the sort of accessibility things you are looking for? @BrianSipple @chuckcarpenter do you guys have any ideas? |
@rwwagner90 Thanks for the reminder 🙂 We've had a few suggestions from our a11y SMEs recently – to be clear, our evaluation is still ongoing and my time to play around with this myself has been limited. A brief summary of what where we've gotten thus far:
Other points that are less clear-cut:
|
One other a11y item - the option to close the tour when clicking outside the tour area or pressing the Esc key (as suggested in #141) would also be helpful for us, though as noted in that issue, those parts at least are feasible to implement on a per-app basis. |
@BrianSipple do we get some of this for free with the conversion to tippy? |
@rwwagner90 Tippy has a minor perk where it adds |
@BrianSipple I think we can do it in 2.x or 3.0. It should be all additional stuff, hopefully nothing breaking. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Do you have any news about this feature? If you have a working draft, I would love to let our sightless user test it and get you some feedback if that helps. |
@knoobie I'm not really sure of what things to do here, but would love to work with you to determine a list of tasks we should implement! |
@rwwagner90 Awesome! Gonna ask my management tomorrow. I'm going to send you a follow up e-mail to your gmail (linked in your GitHub profile) in the next days. Thanks for your quick response! |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Would like to keep this open if possible. |
@gorner we would also love to support this. I think where we need help in the very least of generating the list of items of what that should be. Are the initial items from |
@chuckcarpenter mostly just trying to keep the issue open! We did end up putting shepherd into our Ember app with a bunch of extra jQuery hooks to get the a11y support we needed. We're still looking into how much we'd be able to contribute back. |
@gorner I would love to hear about these extra jQuery hooks. We should definitely incorporate the functionality into Shepherd. This stale bot keeps trying to close things in 30 days, so thanks for commenting to keep things open! If you could share specific things you implemented in small digestible chunks, I would be glad to port them into Shepherd 👍 |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Have updated our project! Need some time to test it properly with our sightless guy. Currently it's summer vacation here |
@rwwagner90 Thanks for the update. I've actually moved on from the company that was using this - sorry for the delay in noting this. Not sure if they'll have bandwidth to test this anytime soon but I'll try looping in @gregmuck just in case. |
Hi, im testing keyboard support, but i think is not stable. Should be optionally disabled, on tour options or on step options. Like:
|
@segux what do you mean it is not stable? I think we need to leave it enabled, for a11y support. |
@rwwagner90 Sorry for late response. Here ill explain what is going wrong on keyboard navigation. I made simple example on stackblitz: Steps to reproduce:
Expected result:
Obtained result:
Probably is not saving the current position of tour. |
@segux I noticed the issues a couple days ago, and fixed them. I was registering the listener in the wrong place, so every time we called |
And what do you think about have properties for total steps number and current step? |
@segux that is a separate discussion, this issue is about a11y support. If you have other things you would like to suggest as features, please feel free to open new issues 😃 |
@knoobie any updates? |
@rwwagner90 summer vacation is 4-6 weeks in my country :) kinda hard to get my coworker in this time |
@knoobie ah okay, I wish I could take 4-6 weeks of vacation in a row! 😃 |
@knoobie when do you think you guys can take a look at this? Just trying to get an idea. I would like to ensure a11y is good to go before we release 4.0 |
@rwwagner90 gonna try to get feedback by the end of next week. |
@rwwagner90 I have seen that you have updated your demo page. Just an idea / note: For sightless people it would be better when the tour can be started by hand (click on a button) instead of "on load" - that's kinda annoying and not intuitive. |
@knoobie it's just the demo page. When implementing in another page it can be triggered however you would like. |
@rwwagner90 I know - we do this. Just as enhancement for your demo to show people other ways of opening it and allowing for better usage of your demo as show case. You know, developer are lazy people ;) |
Got finally some feedback for you:
Nice to have feature:
Didn't tested:
Overall - pretty good upgrade from 2.x 👍 (using correct buttons, better focus handling, trapping the user in the modal / focus circling and so on) |
@knoobie thanks for the feedback!
I'm confused on this. What is not working?
I just made these changes and released 4.1.0, thanks! |
Steps to reproduce: Opening: https://shepherdjs.dev/demo/
You can read more about it here |
@knoobie I get the blue outline on all the elements. The next button takes maybe half a second to show up, maybe due to animations, but I see it. |
@rwwagner90 what Browser did you use? I'm on Firefox 69 (Ubuntu / MacOS) |
@knoobie it appears FF for some reason highlights the text inside the button, instead of the button. It still works though. If you tab to |
I believe most of this has been addressed. If there are additional issues, please feel free to open new ones! Also, @knoobie what is your company? We would love to feature your Shepherd usage in our users 😄 |
I'm part of a team that has been looking at using Shepherd (and ember-shepherd) within one of our externally-facing Ember sites, however we've noticed that the accessibility support is fairly minimal - there are no ARIA attributes on any Shepherd elements and the semantic markup could be improved (no
nav
tag around the navigation buttons, for example).I know there was an issue #26 raised along these lines but it was closed without explanation late last year.
I realize that this is a period of transition in terms of the stewardship of Shepherd but I was wondering if any efforts were being planned in this regard.
The text was updated successfully, but these errors were encountered: