search

backdrop / backdrop-issues

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

[A11Y] Dashboard: incorrect semantic structure in heading tags on the "Welcome to Backdrop" block #5038

Closed ericvanderwal closed 3 years ago

ericvanderwal commented 3 years ago

Issue

The default dashboard for backdrop has incorrect semantic structure in the heading tags.

For example, heading tags should be as such.

etc

Currently "Dashboard" is H1 and each section title is H2 (eg 'Welcome to Backdrop', 'Create Content'). However the subtitles within in section use H4 header tags. Specifically 'Getting Started', 'Next Steps', 'More Actions'

Direct link: /admin/dashboard

Purposed Changes

Change the default block "Welcome to Backdrop CMS", subtitles to be H3 tags instead of H2 tags.

Source The source of this suggestion is generally accepted to be due to rule 1.3.1 "Info and Relationships" of WCAG2.1

(Level A) Information, structure, and relationships conveyed through presentation can be >programmatically determined or are available in text. Link: https://www.w3.org/TR/WCAG21/#info-and-relationships

Image Example image

ericvanderwal commented 3 years ago

Sorry, correction. I meant to say:

Change the default block "Welcome to Backdrop CMS", subtitles to be H3 tags instead of H4 tags.

indigoxela commented 3 years ago

Sorry, correction. I meant to say...

No problem, what you mean is understood. I did a quick look and unfortunately it also requires css changes. Otherwise the subheadings get (wrong) icons applied. Otherwise the change is simple. Only the lack of spare time... :smile:

klonos commented 3 years ago

...unfortunately it also requires css changes.

Thanks for taking the time to report this @ericvanderwal 🙏🏼 ...to understand what @indigoxela meant by this, you may wanna have a quick look at #4167 ...the gist of it is in the issue title and summary.

ericvanderwal commented 3 years ago

Thank you for the update. Yes, without changing CSS, it may be difficult to make certain changes to Basic (Seven?) for accessibility.

indigoxela commented 3 years ago

A PR is available for testing and review. (BTW I don't think this is actually a feature request, changed label to type task.)

ericvanderwal commented 3 years ago

Type task, noted. PR reviewed by me. Looks good.

klonos commented 3 years ago

Type task, noted.

Yup 👍🏼 ...depending on whether one interprets this as a "Incorrect structure" or "Use correct structure", this issue is either a bug (my personal opinion), or a task respectively. Either way, the milestone should be set to the next bug fix release (which is where we merge both tasks and bug fixes), and not the next minor release (which is where new features go).

herbdool commented 3 years ago

The code looks good to me.

ghost commented 3 years ago

Concerns were raised about needing to edit the CSS for this change, but then a PR was made and nothing was mentioned about the CSS changes included... Are these CSS changes ok as-is? What's our stance on CSS changes currently?

indigoxela commented 3 years ago

a PR was made and nothing was mentioned about the CSS changes included... Are these CSS changes ok as-is?

The scope of the CSS changes is very limited. Unlike other changes that potentially apply in many different places (which always has to be handled very carefully), these changes only affect the dashboard. A too greedy selector has been replaced with a more specific one.

If this CSS applies anywhere else, please let me know.

quicksketch commented 3 years ago

Thanks @ericvanderwal for reporting this issue and suggesting the changes! I found you weren't in the triage team yet (able to add labels and edit issues). You should have an invite in your email now.

I've merged https://github.com/backdrop/backdrop/pull/3594 into 1.x and 1.19.x.

CSS changes are still a slippery item for core, but with the extremely limited scope of being specific to dashboard and admin-only, this seems like a relative safe change to make. The less-specific selectors are also a good thing.

Thanks @indigoxela for the PR. And @BWPanda, @herbdool, and @klonos for reviewing. :bow:

olafgrabienski commented 3 years ago

The scope of the CSS changes is very limited. Unlike other changes that potentially apply in many different places (which always has to be handled very carefully), these changes only affect the dashboard. A too greedy selector has been replaced with a more specific one.

Today in the Zulip chat and in the Forum people raised an issue with the look of the icons on the configuration page. I guess, the reason is an unwanted side-effect of the CSS changes mentioned above: the element .admin-panel h3 was removed from the CSS definition; I think it was the element definitin which provided the background-repeat: no-repeat; for the configuration icons.

Starting line of the respective part of the commit: https://github.com/backdrop/backdrop/commit/2c8fb3d115caea9f223d27793f1505b9518b8ce7#diff-e376e77e7a112390744d75e640cc24b43108cea9c527cfcde90290cc705d7a90L1193

indigoxela commented 3 years ago

I see the odd screenshot in the forum, but I can't reproduce it. On a recent 1.19.2 Backdrop everything looks good in Seven.

A severe glitch like that would have been visible when testing, BTW. :wink:

So: we need the steps to reproduce.

indigoxela commented 3 years ago

Update: can reproduce. :disappointed: - it's on a different page.