The mobile navigation button position is incorrect when navigation is expanded.
1. Enable navigation module
2. Go to mobile/tablet screensize
3. Expand the navigation sidebar
4. Expand the right sidebar menu
5. See the position of menu button
6. See the screenshot for reference
Width of the container .site-header__inner__container is calculated incorrectly from layout.css and need to be overridden.
Before
After:
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | After-patch-without-navigation.png | 13.62 KB | mithun s |
| #11 | After-patch-with-navigation.png | 13.94 KB | mithun s |
| #11 | Before-patch-with-navigation.png | 13.94 KB | mithun s |
| #11 | Before-patch-without-navigation.png | 15.58 KB | mithun s |
| #10 | menu-btn.png | 159.87 KB | ahsannazir |
Show commandsStart 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:
changes, plain diff MR !8324
Comments
Comment #4
ahsannazir commentedComment #5
kostyashupenkoGood catch. I can reproduce layout shifting and wrong position of Close button only when `Navigation` module is enabled.
Can be reproduced on screen width around ~1034px
`--drupal-displace-offset-left` is calculating correctly. The real reason of all that is this line https://git.drupalcode.org/project/drupal/-/blob/11.x/core/themes/oliver...
But if to remove this line - we'll get the same bug if `Navigation` module isn't installed (so with default Toolbar).
For now no solution for this, but at least issue summary should be updated
Comment #6
ahsannazir commentedIt seems reseting width fixes the issue. Attaching screenshot for reference.
Comment #7
kanchan bhogade commentedI've tested MR 8324 on Drupal 11
The MR is applied Cleanly...
Testing steps :
1. Enable navigation module
2. Go to mobile/tablet screensize
3. Expand the navigation sidebar
4. Expand the right sidebar menu
5. See the position of the menu button
6. See the screenshot for reference
Test Result:
The mobile navigation button position issue is fixed and it looks fine.
Attaching SS
RTBC+1
Keeping in "Needs review" for Code verification
Comment #8
smustgrave commentedLooking at the issue summary the solution doesn't match the solution so should be documented why the solution fixes the problem.
Please no snippet of the code but the why
Also an after picture should be added the summary also.
Thanks.
Comment #9
ahsannazir commentedResetting/overriding the width works because the width set in layout.css for
body.is-fixed .containeris incorrect for this case.Comment #10
ahsannazir commentedComment #11
mithun sReviewed out the various scenarios for the issue.
1. Without MR being applied and without navigation enabled.
2. Without MR being applied and with navigation enabled.
3. With MR and without navigation enabled.
With MR and with navigation enabled.
The MR looks good, the width for the container needs to be overwritten and after MR the issue seems to be resolving.
Attaching the different screenshots for the same. RTBC +1
Thank you.
Comment #12
mithun sComment #13
smustgrave commented@Mithun S thanks for checking but screenshots were already uploaded in #7.
Thanks @ahsannazir for updating the summary, looks good to me that was the only holdup from before.
Comment #14
nod_Question in MR
Comment #15
ahsannazir commentedComment #16
smustgrave commentedExtra change removed
Comment #18
nod_Committed and pushed 77b8e59604 to 11.x and 103201d3d5 to 11.0.x and 225ed0a05f to 10.4.x and 7c442b0677 to 10.3.x. Thanks!
Comment #23
nod_Comment #24
nod_