search

backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 39 forks source link

[A11Y] Admin bar: Fix various contrast issues #6119

Open AmyJuneH opened 1 year ago

AmyJuneH commented 1 year ago

Color contrast is insufficient - the "error count" button in the dashboard

The "error count" button in the dashboard has insufficient color contrast. "The color of the text and the color of the background are not in sufficient contrast to each other. The contrast ratio should be at least 4.5:1 for normal text and 3:1 for large text. Note: Contrasts are not relevant where content is hidden. But make sure that for hidden content, which becomes visible when receiving focus, the contrast ratio is sufficient. Insufficient contrast between text and its background can give problems for users with low vision and color blindness, but it can also affect a lot of other users, leading to important text potentially being overlooked."

Results: 3.39 out of a required 4.5

eeeeee / rgb(238,238,238)

Background color

ee3d23 / rgb(238,61,35)

avpaderno commented 1 year ago

I cannot find an error count button on the dashboard. Where is it placed? (I take it is always visible.)

laryn commented 1 year ago

I believe @AmyJuneH is referring to the error indicator in the admin bar:

image
AmyJuneH commented 1 year ago

Yes Thank you @laryn

wesruv commented 3 months ago

PR is ready, I fixed a lot of contrast and hover/focus a11y issues (including the one in the ticket): https://github.com/backdrop/backdrop/pull/4804

One test that has nothing to do with CSS failed SimpleTest (Functional Tests) / Simpletest batches (7.4, 3, mariadb-10.3) trying to get them rerun?

laryn commented 3 months ago

I just updated the branch with the latest core commits so the tests will run again.

klonos commented 3 months ago

Thanks @wesruv 🙏🏼 ...I need to take a better look, but as initial feedback:

wesruv commented 2 months ago

I messed around with it further, I like the active item being white with a blue border, got that added.

I noticed the focus state wasn't being caught by the CSS when navigating with keyboard, so I addressed that, and added a 2px outline of the blue color to give a halo to the focused item. That feels more obvious and looks nice.

laryn commented 2 months ago

Keyboard accessibility might be a bigger issue on the admin menu (I can't seem to get the submenus to activate with a keyboard, and the "This page" item gets skipped over completely as shown in the screencast below). That's not part of this issue.

As far as this issue goes, the search box also could get a focus outline.

CleanShot 2024-06-27 at 18 52 44

wesruv commented 2 months ago

@laryn yea, I'm noticing that too. The site is still usable without sub menus, and I think I'd rather change the tech behind the menu than try to fix what's there. I don't think we have any menu JS in Backdrop land that's up to snuff.

Good catch on the search! I'll get that added.

izmeez commented 2 months ago

@wesruv Testing the PR shows the menu items when they reverse direction and overlap other menu items are not readable. admin_bar-color-overlap-unreadable-annotated

wesruv commented 1 month ago

Hrmm, @izmeez I'm not able to reproduce that bug, I double checked, and I shouldn't have changed any layout CSS. I wonder if it was a caching issue or something weird happening?

image

Can anyone else reproduce that issue?

avpaderno commented 1 month ago

I saw the preview site for this PR as @wesruv sees it. I recall I have seen something like @izmeez screenshot in another preview site or a site.

avpaderno commented 1 month ago

I can reproduce it: Changing the browser's window size, when there is not enough space to the right of the menu item, the sub-menu is rendered over the existing menu item, as in @izmeez screenshot.

I do not recall this is specific for this PR. If I remember correctly, it happened also with a different preview site.
I will check what happens with a preview site for a different, maybe newer, PR.

avpaderno commented 1 month ago

I tried with the preview site for https://github.com/backdrop/backdrop/pull/4818.
When there is not enough space to the right of the menu item, the sub-menu item is drawn over the menu item, but it completely covers it. I can clearly read the sub-menu item text.

klonos commented 1 month ago

Thanks for the work on this @wesruv 🙏🏼 ...this looks like a considerable improvement over the current situation 👍🏼

I noticed the focus state wasn't being caught by the CSS when navigating with keyboard, so I addressed that, and added a 2px outline of the blue color to give a halo to the focused item. That feels more obvious and looks nice.

That halo looks nice indeed, but for the "extra" items in the toolbar, it also has a white outline in addition to the blue one you've added:

image

Keyboard accessibility might be a bigger issue on the admin menu (I can't seem to get the submenus to activate with a keyboard, and the "This page" item gets skipped over completely as shown in the screencast below). That's not part of this issue.

I've noticed the same problems while testing this @laryn, and indeed not part of the scope of this issue here.

... I think I'd rather change the tech behind the menu than try to fix what's there. I don't think we have any menu JS in Backdrop land that's up to snuff.

@wesruv we have #2809 for the a11y and keyboard navigation, and now #6669 for the tab index for the "This page" item 😉

Can anyone else reproduce that issue?

Yes @wesruv, I can reproduce it, same as @avpaderno in the previous comments here. On a vanilla installation of Backdrop, on a demo sandbox (without the changes in the PR provided here), the items expand towards the opposite direction, but the top-most item has a background that prevents the text of any underlaying item to blend with the text of the expanded item:

image

...in the PR sandbox, the background is missing, which makes the expanded item become "see-through", and causes the text overlap.

To reproduce, you'll need to reduce the width of your window to a point where the "extra" items are collapsed/hidden away, and only the main navigation items are left. Try to reduce the with to the point just before the main navigation items also collapse. Then, hover over the previous-to-last top-menu item (Configuration), and try hovering over any menu item that goes 3 levels deep (like the "Date and time formats" item that @izmeez mentioned).

Other feedback:

The fixes in the PR have expanded to more than the original bug report here, which warrants a title/scope change I think. I know that there's many people that don't like when that happens, but I'm fine with it in this case here. I really want to see this overhaul get into the next bug fix release.

wesruv commented 1 month ago

Created a follow up PR for the issue @izmeez brought up since it's distinct from this one, and added a fix for it: https://github.com/backdrop/backdrop-issues/issues/6687

wesruv commented 1 month ago

Created a follow up issue for what @laryn noticed: https://github.com/backdrop/backdrop-issues/issues/6688

There some some significant a11y issues there.

Keyboard accessibility might be a bigger issue on the admin menu (I can't seem to get the submenus to activate with a keyboard, and the "This page" item gets skipped over completely as shown in the screencast below). That's not part of this issue.

As far as this issue goes, the search box also could get a focus outline.

stpaultim commented 4 weeks ago

@wesruv - I am a bit confused about if there are additional issues mentioned above that still need to be addressed OR if you have fixed everything you plan to fix in this issue AND moved over problems to other issues?

Does this issue need more testing/feedback now?

izmeez commented 19 hours ago

The PR Tugboat has expired and will need to be rebased. This issue is also related to a new issue #6710