-
Notifications
You must be signed in to change notification settings - Fork 54
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
fix: make prev and next slides not focusable and aria-hidden #2091
Conversation
d057cad
to
a9d1cda
Compare
But it doesn't work on a video slide, as it re-mounts the container and loses attributes. Signed-off-by: Grigorii K. Shartsev <[email protected]>
a9d1cda
to
db36b23
Compare
/backport to stable28 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good and works with NVDA!
Added a test, but wasn't sure where to put it |
|
Signed-off-by: Grigorii K. Shartsev <[email protected]>
01d8ccf
to
34688e0
Compare
/compile amend |
Signed-off-by: Grigorii K. Shartsev <[email protected]> Signed-off-by: nextcloud-command <[email protected]>
34688e0
to
942f0a6
Compare
This comment was marked as resolved.
This comment was marked as resolved.
/backport 36baffb to stable28 |
/backport 6998c9c to stable28 |
The backport to stable28 failed. Please do this backport manually. # Switch to the target branch and update it
git checkout stable28
git pull origin stable28
# Create the new backport branch
git checkout -b fix/foo-stable28
# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123
# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable28 Error: Unknown error More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport |
/backport 6998c9c to backport-2091-stable28 |
(trying something 🙈 ) |
The question in the description is still interesting to me =D |
Resolves
Because of 6d4e7cd the previous and the next slides are rendered and only visually hidden. It makes elements on them focusable and visible for a screen reader.
The previous and the next slide should be inert and hidden.
Notes
Because of forced mixin with
inheritAttrs: force
, it is not possible to pass attributs on the file content element, e.g.<img>
in<Images>
.viewer/src/mixins/Mime.js
Lines 26 to 27 in ea8f76a
So a new wrapper element had to be added.
By the way, it was incredibly hard to find, why attributes doesn't work on components even there is no
inheritAttrs
, no local or globalmixins
. A mixin is added in runtime, mutating a component definition 🥲viewer/src/views/Viewer.vue
Lines 824 to 825 in ea8f76a
Also, in 6b46c8a common
hidden-visually
class was replaced with a customviewer__file--hidden
. I keep it as it is, but I don't understand it. @skjnldsv Do you member why a custom class was required here? How it fixes click outside?Screenshots
No visual changes in other viewers: