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

fix: tab key in code block #4520

Merged
merged 2 commits into from Aug 1, 2023
Merged

fix: tab key in code block #4520

merged 2 commits into from Aug 1, 2023

Conversation

luka-nextcloud
Copy link
Contributor

πŸ“ Summary

πŸ–ΌοΈ Screenshots

🏚️ Before 🏑 After
B A

🚧 TODO

  • ...

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests
  • Documentation (README or documentation) has been updated or is not required

@cypress
Copy link

cypress bot commented Jul 13, 2023

1 flaky tests on run #11449 β†—οΈŽ

0 149 2 0 Flakiness 1

Details:

fix: tab key in code block
Project: Text Commit: 5f28509094
Status: Passed Duration: 03:51 πŸ’‘
Started: Aug 1, 2023 10:01 AM Ended: Aug 1, 2023 10:05 AM
FlakinessΒ  cypress/e2e/sync.spec.js β€’ 1 flaky test

View Output Video

Test Artifacts
Sync > passes the doc content from one session to the next Output Screenshots

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

Copy link
Member

@mejo- mejo- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether we want to do this a11y wise. It will break navigating with Tab if the focus jumps into a code block. CodeMirror has implemented an "escape hatch" for that reason: See https://codemirror.net/examples/tab/

src/components/Editor.vue Outdated Show resolved Hide resolved
@mejo-
Copy link
Member

mejo- commented Jul 29, 2023

@juliushaertl had the idea to add an aria-label attribute to code blocks to make it visible to screen readers that you're inside a code block.

Given that it's very unlikely to end up in a code block by tabbing through the app and that you still can move out of the code block via arrow keys, please disregard my a11y doubts ☺️

@juliushaertl
Copy link
Member

@Pytal @susnux Since you work more on accessibility topics, do you have any hint how how this could be addressed best?

TL;DR: Tab key in a code block should indent but then the tab key can no longer be used to move to the next focus element, arrow keys would work to move out.

@Pytal
Copy link
Member

Pytal commented Jul 31, 2023

As WCAG states "... if it requires more than unmodified arrow or tab keys or other standard exit methods, the user is advised of the method for moving focus away." (https://www.w3.org/WAI/WCAG21/Understanding/no-keyboard-trap.html) this technically meets requirements

But it would be ideal to add an aria-describedby on the code block pointing it to a description informing the user that the arrow keys can be used to move focus out of the code block, any additional recommendations @michaelnissenbaum?

Copy link
Collaborator

@max-nextcloud max-nextcloud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look good to me. Did not test it though.
Maybe accessibility hint could be moved to a separate issue.

@juliushaertl
Copy link
Member

Yes, we can also keep that separate and follow up. Another option might be to just disable the Tab behavior override depending on the user setting for disabling keyboard shortcuts.

Signed-off-by: Luka Trovic <luka@nextcloud.com>
@mejo-
Copy link
Member

mejo- commented Aug 1, 2023

/compile

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@mejo- mejo- merged commit 5cfac93 into main Aug 1, 2023
32 checks passed
@mejo- mejo- deleted the bugfix/tab-on-codeblock-issue branch August 1, 2023 10:18
@mejo-
Copy link
Member

mejo- commented Aug 1, 2023

/backport f1ed024 to stable27

@mejo-
Copy link
Member

mejo- commented Aug 1, 2023

/backport f1ed024 to stable26

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

Successfully merging this pull request may close these issues.

can not using tab in code block
6 participants