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

  1. Install Drupal with standard profile
  2. Add project_messaging to core from MR #2889 (as patch)
  3. Enable core's Announcements Feed module (announcements_feed)
  4. 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.

Issue fork drupal-3334907

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

shaal created an issue. See original summary.

andy-blum made their first commit to this issue’s fork.

gauravvvv’s picture

Status: Active » Needs review
StatusFileSize
new1.22 KB

When announcement off-canvas dialog is open, container is taking the wrong width. I have fixed that and provided the patch, Please review.

gauravvvv’s picture

StatusFileSize
new1.42 KB
new1.33 KB

Fixed CCF, attached interdiff for same

markconroy’s picture

StatusFileSize
new224.05 KB
new227.94 KB

The 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:

Olivero menu

After:

Olivero menu

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative, +Needs issue summary update

This 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.

andy-blum’s picture

Issue summary: View changes
Issue tags: +Needs accessibility review
StatusFileSize
new266.94 KB

Unfortunately, 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

andy-blum’s picture

Version: 10.0.x-dev » 10.1.x-dev

andy-blum’s picture

Status: Needs work » Needs review
Issue tags: -Needs issue summary update
andy-blum’s picture

Title: Mobile menu button is not positioned correctly when side canvas is open » Mobile menu is not positioned correctly when side canvas is open
Issue summary: View changes
guru2023’s picture

Assigned: Unassigned » guru2023

I will review this patch and confirm the same. Is it working fine or not

guru2023’s picture

Assigned: guru2023 » Unassigned
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new202.94 KB
new206.22 KB

Reviewed 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.

rohan-sinha’s picture

Reviewed MR !3490, on comment #9 , issue has been fixed, can be merged.

larowlan’s picture

Adding credits

i don't think this needs an a11y review as its just CSS changes

  • larowlan committed 1d34ad2e on 10.0.x
    Issue #3334907 by andy-blum, Gauravvvv, Guru2023, markconroy, shaal:...

  • larowlan committed be918cb0 on 10.1.x
    Issue #3334907 by andy-blum, Gauravvvv, Guru2023, markconroy, shaal:...
larowlan’s picture

Version: 10.1.x-dev » 9.5.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)
Issue tags: +Bug Smash Initiative

Committed to 10.1.x and 10.0.x

Does not apply to 9.5.x so marking as patch (to be ported)

gauravvvv’s picture

StatusFileSize
new1.17 KB

I 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 into transform: translateX(-100%); /* LTR */ which seems to be an issue for me. I see calc() 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.x

catch’s picture

Version: 9.5.x-dev » 10.0.x-dev
Status: Patch (to be ported) » Fixed

@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.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.