Focus states on mobile second level navigation items can get cut off in Olivero

@mherchel not able to replicate the issue. Could you please provide SS.

I have attached a patch and after patch screenshot for reference. Please review.

The default value for overflow property is visible, so we don't have to add it unless we are trying to override the value. Removing the whole property in the patch.

Re-uploading the patch

Patch #10, works fine. moving to RTBC.

Attached after patch screenshot for reference

Unrelated failure, moving back to RTBC

Verified and tested merge request !1073 - https://git.drupalcode.org/project/drupal/-/merge_requests/1073.patch
Patch applied successfully and looks good to me.

Testing Steps:
# Goto: Appearance -> Apply Olivero theme
# Goto: admin/structure/menu/manage/main?destination=/en/admin/structure/menu
# Add the submenus under any main navigation menu
# Goto Responsiveness devices
# Perform the accessibility and observe the below mentioned results.

Expected Results:
# After applying merge request !1073, Focus states on mobile second-level navigation items should not be cut off. Users should see a proper focus on the respective navigations.

Actual Results:
# Focus states on mobile second-level navigation items can get cut off.

Please refer attached screenshots for the same.
Looks good to me.
Can be a move to RTBC.

The patch adds a lot of more spacing to links in the mobile menu. This can be seen in the screenshots posted in #18. I don't think that's desired, so moving back to needs work.

Applied patch #7 working fine
Thnaks for the patch
For ref sharing screenshots....

Looks like https://www.w3.org/WAI/WCAG21/Understanding/focus-visible.html

I am addressing the comment #19 in this patch, I have made the necessary adjustment to the padding by changing it from var(--sp0-5) to var(--sp0-25), which is the smallest value available. This change successfully resolved the issue related to the “Focus states on mobile second level.” attaching the patch and inderdiff.

I attempted to create a merge request but encountered an error on my local machine. As a workaround, I am creating a patch file in the hopes that this solution will be helpful.

Issue summary should be updated with proposed solution.

Also as UI change before/after screenshots should be added.

1. Update the issue summary with proposed solution,
2. I have tested patch #26 and it is working fine. As requested in #28, I am attaching a screenshot for reference.

Before

After

RTBC +1

Still missing screenshots.

strange, I have checked the images on others system, it's visible on some and breaks on some.
so updated the images again.

Images:
3191727 after patch 26.png
3191727 before patch 26.png

Issue appears to have been resolved. Updated issue summary.

Updated attributions

working on it

Addressing the comment #36, and created a patch along with interdiff. No additional screenshots are being added as they are the same as the ones uploaded in comment #33.

Refactor looks good.

It looks like the padding breaks the close animation. There's a blue border visible during the animation:

working on it

In this patch, I have addressed the comment #42 by adjusting the styling. The issue was that the padding on the ul(primary-nav__menu--level-2) element was causing the animation to break. To fix this, I have removed the padding from the ul(primary-nav__menu--level-2) and instead added padding to the first and last child of the second level items (primary-nav__menu-item--level-2). This solution addresses both the problem of focus cut off and the animation breaking.

Can confirm I'm also seeing the results from #44 applying the patch.

I checked the same patch #44 it is working fine with php 8.3, Drupal 11 requires PHP 8.3.

RTBC looks fine, Can I get the changes in a MR please? Thanks!

Copy to MR seems fine.

Committed and pushed 924d567aac to 11.x and 1e0296d353 to 11.0.x and 4c8f3371e4 to 10.4.x and 57e4625a18 to 10.3.x. Thanks!