Problem/Motivation

Layout builder region on Navigation block page is breaking when collapsed.

Steps to reproduce

1. Install navigation module.
2. Go on admin/config/user-interface/navigation-block/ page
3. Collapse the left side bar.
4. Uncheck 'Show content preview'
5. You'll see the design is breaking.
6. See image for reference

Proposed resolution

Fix the styling of collapsed aside region.

Solution - I have fixed the font size and reduced the padding on nav.

Remaining tasks

User interface changes

After Fix -

API changes

Data model changes

Release notes snippet

Issue fork drupal-3446433

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

Gauravvvv created an issue. See original summary.

sanket.tale’s picture

StatusFileSize
new50.09 KB

Hi @Gauravvvv Could you please provide any styling recommendations? Here's how I styled it and will modify it if necessary.

ahsannazir’s picture

StatusFileSize
new151.63 KB

i could think of reducing the font-size to 1rem but still it the labels are not fully visible due to very less space available.
Attaching screenshot how it looks with font-size of 1rem.

sanket.tale’s picture

Hi, Created MR for the issue please review it. Thanks!

sanket.tale’s picture

Status: Active » Needs review
smustgrave’s picture

Status: Needs review » Needs work

Issue summary should be updated to include the solution and after screenshots.

Don't need a code snippet but what was actually fixed.

ahsannazir’s picture

StatusFileSize
new153.92 KB

The layout builder section seems fixed in collapsed state by reducing the font-size and paddings. Attaching screenshot|

sanket.tale’s picture

Issue summary: View changes
StatusFileSize
new57.13 KB
sanket.tale’s picture

Status: Needs work » Needs review
kanchan bhogade’s picture

StatusFileSize
new85.21 KB
new85.84 KB

I've tested MR 8602 on Drupal 11.x
MR is applied cleanly...

Testing steps:
1. Install the navigation module.
2. Go to admin/config/user-interface/navigation-block/ page
3. Collapse the left sidebar
4. You'll see the design is breaking.
5. Appy MR and check for the same

Test Result:
The layout builder section is fixed in the collapsed state and visually looks good.
Attaching SS for reference
RTBC+1

Keeping "needs review" for code verification

bnjmnm’s picture

Status: Needs review » Needs work
StatusFileSize
new220.62 KB

it seems like the issue reporter and everyone working on this issue are getting a version of the sidebar that is providing the alt text instead of the icons. This is not something I'm able to reproduce, but considering multiple contributors are seeing the exact same thing, I have to assume there are fairly straightforward steps beyond those in the issue summary to and this should be documented. I'm also not sure how to get it where layout builder can be used on a navigation block as seen in the screenshots, so that would be good to include in the issue summary as well.

If the icons aren't showing up, there are already problems happening and making this look nicer isn't bad but probably not a major priority. We should ensure nothing in this MR conflicts with the correctly-loading Navigation bar.

I see several other issues with screenshots of the nav sidebar looking like it does here, so I'm guessing this isn't an icon-loading problem like I initially thought. However, I'm not able to access this area using the steps in the issue summary - could this be expanded with a little more info on how to get to the navigation block layout editor?

Disregard - I had a pre-core navigation module in contrib which gets prioritized over ones in core.

finnsky’s picture

Imo simplest option here will be always expanded sidebar on that page(on desktop). ignoring user setting.

ahsannazir’s picture

@finnsky Does #13 mean that when "show content preview" is unchecked we have to expand the toolbar even it is already in collapsed state?

quietone’s picture

Version: 11.0.x-dev » 11.x-dev

Fixes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

ahsannazir’s picture

Status: Needs work » Needs review
finnsky’s picture

Status: Needs review » Needs work

Added 2 comments.

ahsannazir’s picture

Status: Needs work » Needs review
ahsannazir’s picture

finnsky’s picture

Status: Needs review » Needs work
nayana_mvr’s picture

Issue summary: View changes
StatusFileSize
new298.8 KB
new285.63 KB
new743.95 KB
new8.76 MB

I have verified the changes in MR !8602 and it fixes the issue mentioned in the ticket. Please find the before and after screenshots.
As suggested by @finnsky, I tried to place the layout builder related code change of core/modules/layout_builder/css/layout-builder.pcss.css in core/modules/navigation/css/base/layout-builder.pcss.css. But the styles are getting overridden and I'm not sure !important should be used in such cases. Is it really necessary to move this code?

3446433

Regarding

this is another (design) problem. not strictly required here imo

, I also feel like it can be removed from this MR as it is not relevant for this issue and also it is causing misalignment of logo on collapsed mode (Please see the screen recording)

finnsky’s picture

But the styles are getting overridden and I'm not sure !important should be used in such cases. Is it really necessary to move this code?

!important definetly should NOT be used by anyone anywhere anytime ;)

- you can remove :where and it will increase CSS specificity

nayana_mvr’s picture

Status: Needs work » Needs review
StatusFileSize
new294 KB

Thanks! @finnsky for the suggestion. I removed :where and added .navigation-layout to increase CSS specificity because even after removing :where, style was getting overridden . Also, I have removed admin-toolbar.css changes as per review comment. Please see the attached screenshot as a reference to these changes.

fix

Kindly review.

finnsky’s picture

Status: Needs review » Reviewed & tested by the community

LGTM. Thank you!

quietone’s picture

I didn't find any unanswered questions here. I can't comment further on CSS issues. Leaving at RTBC

  • nod_ committed c05d7345 on 10.3.x
    Issue #3446433 by sanket.tale, ahsannazir, nayana_mvr, gauravvvv,...

  • nod_ committed 9be6e1c3 on 10.4.x
    Issue #3446433 by sanket.tale, ahsannazir, nayana_mvr, gauravvvv,...

  • nod_ committed 0d81ddd1 on 11.0.x
    Issue #3446433 by sanket.tale, ahsannazir, nayana_mvr, gauravvvv,...

  • nod_ committed ba3a786b on 11.x
    Issue #3446433 by sanket.tale, ahsannazir, nayana_mvr, gauravvvv,...
nod_’s picture

Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed ba3a786beca to 11.x and 0d81ddd1910 to 11.0.x and 9be6e1c3cc7 to 10.4.x and c05d734567b to 10.3.x. Thanks!

Status: Fixed » Closed (fixed)

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