Problem/Motivation
Opening the offcanvas dialog in combination with Olivero's mobile nav can move the menu button outside of it's visual container

Steps to reproduce
- Install Drupal with standard profile
- Add project_messaging to core from MR #2889 (as patch)
- Enable core's Announcements Feed module (announcements_feed)
- While logged in as admin, open Olivero slide-out nav, then click "Announcements" in the admin toolbar
Proposed resolution
Displace the mobile navigation when the offcanvas dialog is opened.
| Comment | File | Size | Author |
|---|---|---|---|
| #19 | 3334907-9.5-19.patch | 1.17 KB | gauravvvv |
| #13 | after-patch-apply-screenshot.png | 206.22 KB | guru2023 |
| #13 | before-patch-apply-screenshot.png | 202.94 KB | guru2023 |
Issue fork drupal-3334907
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
Comment #3
gauravvvv commentedWhen announcement off-canvas dialog is open, container is taking the wrong width. I have fixed that and provided the patch, Please review.
Comment #4
gauravvvv commentedFixed CCF, attached interdiff for same
Comment #5
markconroy commentedThe patch applies cleanly, and seems to fix the issue. However, it does create a very small layout shift.
If Olivero maintainers are okay with this small shift (it's about 6px), I think this could be marked RTBC.
Before:
After:
Comment #6
smustgrave commentedThis issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.
This could use an issue summary update. Proposed solution is empty, obviously the goal is to fix it but how was it being fixed?
Before/after screenshots should be added to the IS also.
Comment #7
andy-blumUnfortunately, I think this approach will not work. Between the range of screen widths that show the mobile menu (including the desktop version in some configurations), and the resizability of the off-canvas menu, I don't think we want the menu button to move at all, unless the entire slide-out menu moves with it to avoid situations where the menu button leaves it's visual (but not DOM) container as in the screenshot below (patch #4 applied along with MR #2889 from [
#3206643])
Because of the interactions between the offcanvas dialog and the navigation, I'm adding the needs accessibility review tag
Comment #8
andy-blumComment #10
andy-blumComment #11
andy-blumComment #12
guru2023 commentedI will review this patch and confirm the same. Is it working fine or not
Comment #13
guru2023 commentedReviewed the patch #4 and tested it on Drupal version 10.1.x. The patch works fine and I have added the before and after screenshot for reference.
Comment #14
rohan-sinha commentedReviewed MR !3490, on comment #9 , issue has been fixed, can be merged.
Comment #15
larowlanAdding credits
i don't think this needs an a11y review as its just CSS changes
Comment #18
larowlanCommitted to 10.1.x and 10.0.x
Does not apply to 9.5.x so marking as patch (to be ported)
Comment #19
gauravvvv commentedI have added patch for 9.5.x. but I see a problem while compiling the patch. This CSS code
transform: translateX(calc(-100% - var(--drupal-displace-offset-right, 0px))); /* LTR */is compiled intotransform: translateX(-100%); /* LTR */which seems to be an issue for me. I seecalc()function is calculating on the compilation time only, noticed that for other units as well. this is happening in 9.5.x only not on 10.1.xComment #20
catch@Gauravvv that is probably due to Drupal 9's supported browsers. If this is complex to backport, I think we should leave it in 10.0.x.