Add account permissions hint for screen reader users #4902
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| it "adds a hint when an action cannot be performed" do | ||
| render_inline Account::PermissionsListComponent.new(User.new) | ||
|
|
||
| expect(page).to have_css "li", exact_text: "Additional verification needed Support proposals", normalize_ws: true |
There was a problem hiding this comment.
Layout/LineLength: Line is too long. [117/110] (https://rubystyle.guide#max-line-length)
javierm
force-pushed
the
permission_list_accessibility
branch
2 times, most recently
from
608831b
to
49b319e
Compare
javierm
force-pushed
the
permission_list_accessibility
branch
from
49b319e
to
b859cb8
Compare
javierm
force-pushed
the
permission_list_accessibility
branch
from
b859cb8
to
830501c
Compare
We were using similar code in four different places; six, if we count the welcome pages seeds. Reducing duplication in the pages seeds is a bit tricky because administrators are supposed to edit their content and might remove the HTML class we use to define styles. However, we can share the code everywhere else. Note that there's a bug in the application since we show that level 2 users cannot vote for budget projects but we give them permission to do so in the abilities model. We're keeping the same behavior after this refactoring but we might change it in the future.
This way we simplify the ERB code. Due to the bug mentioned in the previous commit, we're keeping the original code instead of using `can?` to check permissions.
As mentioned in commit 925f04e, icon classes make screen readers announce strange symbols and aren't properly displayed for people who have changed their preferred font family.
We were displaying an icon showing that certain actions can't be performed. However, people who can't see the icons were hearing that they _can_ perform certain actions while the opposite is true. We've considered other options to solve this problem. One was to split the list in two: actions that can be performed and actions that can't be performed. It was tricky because in some cases we're listing that actions that can be performed now and in other cases we're displaying the actions that people will be able to perform once they verify their account. Another option was to include the word "Cannot" as a prefix instead of "Additional verification needed". We haven't done so because, while in English we say "cannot do this thing", in other languages they say "this thing cannot do". So we've gone with a solution where people hearing what's on the screen know what's going on and we don't have to make big changes in the code.
javierm
force-pushed
the
permission_list_accessibility
branch
from
830501c
to
2bac802
Compare
taitus
approved these changes
Dec 1, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objectives