focus state outline can get cut off certain situations

I have attached a patch for the issue and added after-patch screenshots for reference. Please review.

+++ b/core/themes/olivero/css/components/navigation/nav-primary.pcss.css
@@ -147,7 +147,7 @@
-  overflow: hidden;
+  overflow: visible;

The overflow property has the default value of visible, so we can remove the property unless we are overriding the value.

Updated patch as per comment #5.

Applied patch #6. Got same issue for Home link in breadcrumb. After applied patch, there is no change in breadcrumb. Please check the screenshots.

Tested Patch #6, Including screenshot to show the patch is working.

@mherchel Please check below findings related to this issues,

+++ b/core/themes/olivero/css/components/breadcrumb.pcss.css
@@ -52,7 +52,6 @@
-  overflow-x: auto;

We can remove this overflow because its outer wrapper(.breadcrumb__content) also having overflow auto so using that breadcrumb will work same as expected.

+++ b/core/themes/olivero/css/components/navigation/nav-primary.css
@@ -284,7 +284,6 @@
  overflow: hidden;

Instead of removing overflow here we can add 3px padding(top & bottom) to active child(.primary-nav__menu--level-2.is-active-menu-parent) which will resolve child menu focus issue and even transition will remain as it is.

Here I am attaching a screen video please have a look. So that we can go ahead or think of another approach.

Thanks!

@mherchel Here I am providing another solution to this issue.

Kindly review the attached patch and feel free to add your opinion on this.

Here I am attaching a screen video please have a look. So that we can go ahead or think of another approach.

Note: There are some diff changes of the semi-colon(;) removal in nav-primary.css which is an auto process for the single property we can ignore that.

Thanks!

Since custom commands failed, here providing an updated patch after fixing drupalci issues. Rest other things are fine and as per #18 description.

Kindly review the attached updated patch.

Thanks!

Just Re roll the patch 9.3.x

As per the @mherchel last comment. so we have keep it overflow: auto and removed overflow-x property.

patch #20 working as expected.

@kiran.kadam911 Thanks for the patch.

Verified and tested patches #20 and #24.
Patch applied successfully and looks good to me.

Testing Steps:
# Goto: admin/config/development/performance
# See Breadcrumbs
# Check the focus
# User should see focus state outline should be accurate.

Expected Results:
# User should see focus state outline should be accurate on a element.
# User should see The parent selector has overflow: auto
# User should not see overflow-x property

Actual Results:
# User is able to see overflow: auto in Child selector
# User is able to see focus state outline is getting cut off.

As per the @mherchel last comment. so we have kept it overflow: auto and removed the overflow-x property.

Can be a move to RTBC

@lauriii and I have discussed this issue. It would be great if the issue summary was updated to contain steps to reproduce the issue and define the entire scope. There seems to be overflow issues for both primary menu and breadcrumbs - can the issue summary have steps for both situations.

Also the latest changes from #18 onwards significantly changed the solution - it would be great to have a detailed explanation of why we moved from a change that only changes overflows to a more complex solution.

+++ b/core/themes/olivero/css/components/navigation/nav-primary.css
@@ -152,19 +184,19 @@
-  padding-left: 0;
+  padding-left: 0
...
-  padding-right: 0;
+  padding-right: 0
...
-  padding-right: 0;
+  padding-right: 0
...
-  padding-left: 0;
+  padding-left: 0

All these removals of semi-colons is very odd. Seems to be a bug in the build system. I think we should land #3246141: Update JavaScript dependencies prior to 9.3.0 first and see if it fixes it.

Applied #20 and #24 patch working fine giving expected result
for ref sharing screenshot ....
Thanks for the patch

Tagging for https://www.w3.org/WAI/WCAG21/Understanding/focus-visible.html

Hi
I checked both issues in 11.x-dev.

Mobile - Primary menu submenus is fixed in checked version
Mobile - Breadcrumbs - Issues still exists.

I have created patch for it. Adding before and after screenshots for reference.

I have created Patch file as I am facing issue in creating MR (new branch).

Hi,

I checked both issues on the 11.x-dev version. The breadcrumb display on mobile was not working correctly, but it’s functioning well after applying the patch. However, I couldn’t reproduce the primary issue; it works fine without any patch. So I’ve created a merge request for patch #33.

Have not reviewed, issue summary is incomplete.

Updated the issue summary. Moving to NR.

Hello,
I have tested and verified this issue on Drupal 11.x version with Oliviero.
MR applied successfully.

Testing steps:
Step1 : Install Drupal use Olivero as default theme
Step2 : Add the breadcrumb block to the page or content type nodes.
Step3 : Visit the pages via the Mobile/Tabs
Step4 : Click the breadcrumb link and observe the outline highlight.

Attached the Before and After Screenshot for Mobile/Tab.
Hence can be move to RTBC
RTBC+1

Reviewed the MR RTBC+1

Based on the review from #46 updating issue summary with before/after screenshots.

Saving credit for the review in #46 also.

Thanks for sorting out the credits!

left a question

The Olivero theme was approved for removal in #3590816: [policy, no patch] Deprecate Olivero and move to contrib.

This is Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.

The deprecation work is in #3595082: [meta] Tasks to deprecate the Olivero theme and the removal work in #3595085: [meta] Tasks to remove the Olivero theme.