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

refactor(Accessibility): improve patch options #1369

Merged
merged 5 commits into from
Oct 16, 2023

Conversation

validcube
Copy link
Member

@validcube validcube commented Oct 13, 2023

Tooltip

Improve accessibility, and avoid user from accidentally resetting their patch options

image

Colour

Changed from Secondary Container to Surface to provide slightly better hierarchy.

image

Ignore commit message, something messed up

Yes, my budget for writing PR has temporarily dropped :trollface:

@validcube validcube changed the title refactor(Accessibility): Tooltip for resetting patch options refactor(Accessibility): tooltip for resetting patch options Oct 13, 2023
@validcube validcube marked this pull request as draft October 13, 2023 11:28
@validcube validcube requested a review from PalmDevs October 15, 2023 10:01
@validcube validcube marked this pull request as ready for review October 15, 2023 10:01
@validcube validcube changed the title refactor(Accessibility): tooltip for resetting patch options refactor(Accessibility): improve patch options Oct 15, 2023
assets/i18n/en_US.json Outdated Show resolved Hide resolved
Copy link
Member

@PalmDevs PalmDevs left a comment

Choose a reason for hiding this comment

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

Needs string changes.


Another problem with this page is the dialog. Currently, the "list item(s)" don't look like list items at all. It doesn't really tell the user it's clickable and is a choice.

What we could do is revert to the default list item styles and add a dummy radio button which will do absolutely nothing other than convince the user the list item is clickable like in the image below.

The behavior won't change, clicking on the list item won't make it selected, it will add the clicked option automatically.

Image example

@PalmDevs
Copy link
Member

@oSumAtrIX What do you think about the solution I've proposed in the review?

@validcube
Copy link
Member Author

validcube commented Oct 15, 2023

Another problem with this page is the dialog. Currently, the "list item(s)" don't look like list items at all. It doesn't really tell the user it's clickable and is a choice.

What we could do is revert to the default list item styles and add a dummy radio button which will do absolutely nothing other than convince the user the list item is clickable like in the image below.

The behavior won't change, clicking on the list item won't make it selected, it will add the clicked option automatically.

Image example

👋 Hello!

Unfortunately, this is out-of-scope for this PR as this is targeted to improve usage accessibility tools like Android TalkBack, screen reader. (and small thing like contrasts)

Feel like to open an issue about this.

@PalmDevs PalmDevs dismissed their stale review October 15, 2023 11:22

out of scope

Copy link
Member

@PalmDevs PalmDevs left a comment

Choose a reason for hiding this comment

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

LGTM

@validcube validcube merged commit ccc6be1 into ReVanced:dev Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants