This is copied over from @rkoller's Gin issue #3541719: Adjust the top bar to the landmark used with the navigation module, and verified by me, Mike, Jurgen, and Bernardo

Problem/Motivation

#3452724: Navigation side bar and top bar should have appropriate aria labels solidifies the landmarks used with the navigation module. With the MR applied to core and using Claro you have two landmarks, Administrative sidebar navigation for the navigation sidebar and Administrative top bar complementary for the top bar.
the node edit form in claro with the voiceover rotor visible showing the landmarks section
While in Gin you only have the landmark for the navigation sidebar, the one for the top bar is missing:
the node edit form in gin with the voiceover rotor visible showing the landmarks section

Steps to reproduce

Proposed resolution

i would align with a few details in claro:

  • change the wrapping element from a div to an aside to make it more semantic
  • add a visually hidden h3 and use that h3 as the label for the aside

the naming of both landmarks for the navigation sidebar and top bar might change at a later point, see #3539715: Review navigation landmark naming

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork drupal-3583486

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mherchel created an issue. See original summary.

mgifford’s picture

bernardm28’s picture

StatusFileSize
new265.12 KB

imagine without a gin file

Looks like just removing top-bar--gin.html.twig would create some chaos with dark mode.

bernardm28’s picture

The image above wasn't the best example because I was confusing the primary aside.
However, here is a better one.

Without the gin toolbar override. In other words, what happens if we delete that file.
low contrast

Removing the gin toolbar creates color contrast issues with the submenus

With gin override.

with gin override

That said, if all we need for the MVP of this issue is to move a step forward. The PR above is similar to the one on the gin project and provides us with a more consistent experience.

bernardm28’s picture

Status: Active » Needs review
bernardm28’s picture

I should add the main color issue lies on the css class gin--navigation-top-bar missing if we remove the twig override.
Other than that DDEV VRT fails because removing the file also gets rid of the gin_breadcrumbs so idk if we are ok losing those.

If we merge the PR above then we don't have to take care of those issues yet and then most of the playrights test pass and the only that failed seem to be a false positive.

mgifford’s picture

I suspect that the color issue should be addressed elsewhere. This should be focused on adding back in the landmarks.

I'd love to the landmark brought into the theme.

kentr’s picture

RE #7:

Creating color contrast issues and / or losing breadcrumbs would be regressions.

Some code for dark mode will be needed regardless. Personally, I think that should remain in the theme.

smustgrave’s picture

Status: Needs review » Needs work

If we can move the color issue to a separate issue?

kentr’s picture

mgifford’s picture

I added a follow-up issue to address the color issues that @bernardm28 mentioned in #7

https://www.drupal.org/project/drupal/issues/3611446

@smustgrave the patch associated with this issue doesn't have any color changes in it. We should be able to assess these separately now.

mgifford’s picture

Status: Needs work » Needs review
needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

kentr’s picture

Issue tags: +Needs rebase

I think it needs a rebase.

kentr’s picture

I'll rebase it.

kentr’s picture

Rebased.

To me, the IS needs updating because it references Gin and the screenshot of the problem doesn't match what I see with a fresh install on main (for example, there are no "Edit", "Preview", "Delete" buttons).

jurgenhaas made their first commit to this issue’s fork.

jurgenhaas’s picture

Issue fork got rebased.

kentr’s picture

Issue tags: +Needs reroll

I think the name of the template file changed in main to top-bar--default-admin.html.twig.

Alternate STR:

  • Log in as admin.
  • Go to /user/1/edit.
  • Run a scan with the Axe browser extension with best practice rules enabled.