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] Keyboard navigation in security passcode view #4455

Merged
merged 5 commits into from
Aug 26, 2024

Conversation

Aitorbp
Copy link
Contributor

@Aitorbp Aitorbp commented Aug 20, 2024

Related Issues

App: #4447

  • Add changelog files for the fixed issues in folder changelog/unreleased. More info here
  • Add feature to Release Notes in ReleaseNotesViewModel.kt creating a new ReleaseNote() with String resources (if required)

QA

@Aitorbp Aitorbp self-assigned this Aug 20, 2024
@Aitorbp
Copy link
Contributor Author

Aitorbp commented Aug 20, 2024

The toolbar is not accessible by the keyboard because of technical restriction. See this comment to more details.

@Aitorbp Aitorbp linked an issue Aug 20, 2024 that may be closed by this pull request
8 tasks
@Aitorbp Aitorbp force-pushed the feature/keyboard_navigation_in_secutiry_passcode_view branch from c8693bd to 7d67070 Compare August 21, 2024 08:34
@Aitorbp Aitorbp requested a review from jesmrec August 21, 2024 08:35
@jesmrec
Copy link
Collaborator

jesmrec commented Aug 22, 2024

Things i am missing here:

  • Navigation through buttons is correct, but, numerical keyboard should work as well. User should be able to type/delete the passcode with the physical keyboard in the same way as clicking buttons.

  • Navigation through buttons is a keyboard trap in the settings. I did not find the way to jump from the buttons to the topbar, in case i want to cancel the view

  • Are the focused buttons with the keyboard fulfilling contrast requirements?

@Aitorbp
Copy link
Contributor Author

Aitorbp commented Aug 22, 2024

Things i am missing here:

  • Navigation through buttons is correct, but, numerical keyboard should work as well. User should be able to type/delete the passcode with the physical keyboard in the same way as clicking buttons.

Now It should work.

  • Navigation through buttons is a keyboard trap in the settings. I did not find the way to jump from the buttons to the topbar, in case i want to cancel the view

Try again. With my external keyboard the ESC key is working. But I added something else. Let´s see.

  • Are the focused buttons with the keyboard fulfilling contrast requirements?

No, It is not meeting the contrast requirements
@jesmrec

@jesmrec
Copy link
Collaborator

jesmrec commented Aug 23, 2024

After last commit:

  • typing numbers with physical keyboard is now posible. Numbers are displayed while typing. It'd be better not to be shown but this is not a big problem

  • Esc works to dismiss the view 💯

@Aitorbp
Copy link
Contributor Author

Aitorbp commented Aug 26, 2024

About contrast requirements:
Here we have the same problem that we had in this issue: #4369
If we change the focus color to black, the only contrast requirement that the 4.5:1 meets is the blue of the button with the white of the view. The other colors do not meet this contrast requirements.

Captura de pantalla 2024-08-26 a las 10 32 35

If we do the same test with the gray of the background, this would be the result:

Captura de pantalla 2024-08-26 a las 10 39 39

On the other hand, the contrast between the focused and unfocused state does not meet the recommended contrast ratio either (at least 3:1). For example, if we focus on the blue of the buttons:

Captura de pantalla 2024-08-26 a las 10 37 51

To give you an idea of ​​what the final result would be in the application with the focus to black:

Captura de pantalla 2024-08-26 a las 10 37 51

@jesmrec
Copy link
Collaborator

jesmrec commented Aug 26, 2024

Let's move the contrast thing to the global topic, then...

this one is approved, at least for a first iteration.

@Aitorbp Aitorbp force-pushed the feature/keyboard_navigation_in_secutiry_passcode_view branch from a2b7ea3 to e3898c2 Compare August 26, 2024 11:28
@Aitorbp Aitorbp merged commit 4c43994 into master Aug 26, 2024
7 checks passed
@Aitorbp Aitorbp deleted the feature/keyboard_navigation_in_secutiry_passcode_view branch August 26, 2024 12:10
@JuancaG05 JuancaG05 changed the title [ally] Keyboard navigation in security passcode view [a11y] Keyboard navigation in security passcode view Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[a11y] Keyboard navigation in security passcode view
2 participants