joomla-cms
joomla-cms copied to clipboard
joomla
Draft - [5.2.] Fix for Guidedtours Calendar Items - fix Issue #43671
Pull Request for Issue #43671 .
Summary of Changes
I am neither sure if the Javascript I added os ok or at the correct position in code. The only thing is that it works ;) Maybe @obuisard can direct me if something needs to be changed there to be "the correct way"
Testing Instructions
Follow instructions from https://github.com/joomla/joomla-cms/issues/43671
Actual result BEFORE applying this Pull Request
The calendar Popup is not accessible
Expected result AFTER applying this Pull Request
The calendar Popup is in the front
Link to documentations
Please select:
-
[ ] Documentation link for docs.joomla.org:
-
[x ] No documentation changes for docs.joomla.org needed
-
[ ] Pull Request link for manual.joomla.org:
-
[ x] No documentation changes for manual.joomla.org needed
@coolcat-creations Which base branch would be right? You can change it on GitHub as described by @Quy , or I can do that for you if I know the right branch.
I have looked into it and tested the change. The PR goes in the right direction.
Although I agree this is mostly a problem of z-index, I am not certain the class change is necessary, as I see no difference with or without it (unless I missed something).
The PR fixes the issue for mouse clicks but it is not accessible. As soon as the arrow keys are used to change the dates in the calendar, the tour step buttons change focus and the tour starts to 'navigate itself away' from the step, going to next or previous steps. So, in a way, we need to 'block' events associated with the tour until the date is selected.
I can also see that the calendar buttons are not reachable through keyboard keys (I do not know what combination to use to access 'today, 'close' ...). But it is not a problem associated to the tours.
Besides that, this PR has JavaScript code style errors. See the log here: https://ci.joomla.org/joomla/joomla-cms/79465/1/20
Yes it's not ready to be tested rather put for discussion.
Please change the PR to a draft then
@obuisard Thank you, I removed adding the class because you are right, it does not change anything. Also, I did not mean accessible by terms of accessibility, but to be able to click into the calendar. I disabled the navigation through the tour when the calendar is open but after that step the tour is broken and the steps are not shown at their correct position, why? Is the way I solved the problem and the place where I did it correct? @richard67 I fixed the lint errors too :-)
@obuisard Thank you, I removed adding the class because you are right, it does not change anything. Also, I did not mean accessible by terms of accessibility, but to be able to click into the calendar. I disabled the navigation through the tour when the calendar is open but after that step the tour is broken and the steps are not shown at their correct position, why? Is the way I solved the problem and the place where I did it correct? @richard67 I fixed the lint errors too :-)
Thank you for the PR Elisa. As we get deeper into building tours, we hit a few limitations and I am hopeful we can 'fix' those limitations over time. As far as the latest modification you made for disabling tour keyboard events, I still have to look into it to see why it messes up the tour.
Thank you for looking into it, The calendar a11y is another already open issue :)
Maybe we should revisit the use of arrow keys in Shepherd (the library the guided tours are based on)... https://github.com/shepherd-pro/shepherd/issues/1873#issuecomment-1119405892
Any news on this one?
I believe we won't be able to fix this one, the calendar field is not accessible itself.
