Olivero: Focus outline does not accommodate long text in secondary nav bar

Addressed #15, attached patch and interdiff. please review

Applied patch #18 successfully and it fixes the issue. Adding screenshots for reference.

I have reviewed Patch #18 but It's not working for me on hover and on focus, Adding screenshots for reference.

After patch:

I have added after patch screenshot.

I have reviewed Patch #18 but It's not working for me on hover and on focus, Adding screenshots for reference.

You have to clear the cache. This might be a cache issue in your screenshot.

Yes @Gauravvvv,
Now working fine Patch #18 its was cache issue. Attached screenshot after clear cache.

Verified issue and that patch #18 addresses issue.

Not attaching screenshots as that was done in #20 and #21

#18 No longer applies, probably due to changes in HEAD since the last RTBC

The reroll can be self-rtbc'd without another review cycle, just spot check that the same files are changed and the file size is similar.

Please review.

Reroll seems fine.

There's some changes to the primary menu in here, also. Not sure if that's intentional.

I'm going to look through this a bit more thoroughly tomorrow.

This patch is looking pretty good, but has a couple issues:

• The comment above. I'm not sure what that rule accomplishes.
• This issue fixes the same problem on both the primary and secondary menus. The issue summary says it just fixes it on the secondary menu. We need to update it.
• Previously the height of the focus outlines between the primary and secondary menus were equal. Now the secondary menu's focus state has a shorter height.

Thanks for all the work on this!

Updated the issue title as per #33 comment.

As per comment #33, I have created another patch to address the above issue. I am attaching a screenshot for reference, showing the state before and after applying the patch.

Fixed build error, attached interdiff for same. please review

Please don’t assign tickets to yourself unless a maintainer. A simple comment should be good.

Seems not all the points from #33 were addressed or answered to not be included.

Hi

Based on comment #33, I tested all three points using the patch provided in comment #36 and found the following:

1. In the Merge Request !3986 and the patch #36, the 'align-items: center' property was added to the 'region-secondary-menu.pcss.css' file. However, after applying the patch, there were no visible changes in the user interface (UI). so removing this in 3238618-40 patch.

2. The issue summary needed an update, but only the issue title was updated in comment #34, the issue summary was also updated with the comment.

3. The height of the focus outlines between the primary and secondary menus was unequal before, but the issue has been resolved by applying the patch provided in comment #36. screenshot was attached to the comment for reference.

Before:

After:

The last submitted patch, 40: 3238618-40.patch, failed testing. View results →

patch test is passed in #40, so restore the status to Need Review.

believe points from #33 have been addressed.

The last submitted patch, 40: 3238618-40.patch, failed testing. View results →

Unrelated failure.

The last submitted patch, 40: 3238618-40.patch, failed testing. View results →

Seems unrelated

The last submitted patch, 40: 3238618-40.patch, failed testing. View results →

Restoring the status as the patch has unrelated failures.

I'm triaging RTBC issues → . I read the IS, the comments and the MR.

Not all the points in #33 have been addressed. See https://git.drupalcode.org/project/drupal/-/merge_requests/3986/diffs#no.... I went back and skimmed the comments. The problem is that the MR and the latest patch are not the same. The MR needs to be updated with the latest, correct changes.

This also needs a title change that is not simply a problem statement. Something like, "Allow primary & secondary navigation focus outline to work with long text"

I hid all files except this initial image and the latest before/after screenshots.

I restored the issue summary so that a Remaining tasks section was available. Please keep all sections of the issue summary template to help reviewers and everyone keep track of the work on an issue. Thanks.

So, just a few things to tidy up here.

Committed changes from patch 40 to the MR.
Updated issue title.

Please let me know if there are any problems with these changes.