The new navigation should use the Drupal displace API to inform themes that it exists, and what it’s width is. This lets themes fix position elements, without overlapping the HTML elements.

More information on the displace is at https://www.drupal.org/node/1956804

Issue fork navigation-3378754

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

mherchel created an issue. See original summary.

claireristow’s picture

Assigned: Unassigned » claireristow

Working on this now!

claireristow’s picture

Hey @mherchel, I'm struggling to find a way to make Drupal.displace work with CSS transitions. Since the sidebar width transitions when collapsed/expanded, the main page content position can't be calculated until the transition is complete, causing a delay/glitch. Check out the tugboat for the visual. I'm wondering if you have any insight on this one?

claireristow’s picture

Assigned: claireristow » Unassigned
Status: Active » Needs work
mherchel’s picture

Status: Needs work » Needs review

Just pushed some changes that should resolve the issue. The remaining bug is if the page is loaded with the navbar in the wide state, and then switched to the narrow state, the transition will does not apply on the very first state change. Other than that, it looks good IMO.

ckrina’s picture

Closing #3380683: The sidebar in umami appears on top of the content in favor of this one. But moving here the discussion:

I mentioned:

Yeah, that's a really good question. In "theory" yes per https://api.drupal.org/api/drupal/core%21modules%21system%21templates%21..., but the reality is that a lot of front-end themes want to have full control of their markup which as can be seen in the off-canvas change record: #2896596: New Off-Canvas dialog tray now available.

Maybe it would be a good thing to discuss it with @lauriii as the old Front-end framework manager that might have enough context of what happened, @bnjmnm (and the rest of FFM) and other experienced devs to see what can be the best solution.

I'd personally think yes, we should rely on having the standard off canvas and if somebody wants to customize it they can do it, but this is would definitely be a breaking change and wouldn't let us do it in a minor? Let's discuss it :)

See the comments it triggered in the Change Record for the New Off-Canvas dialog: #2896596: New Off-Canvas dialog tray now available

Also, @bnjmnm mentioned this as FEFM:

Ideally Umami would leverage off canvas offsets since it's used as a reference for how to fully take advantage of Drupaly things. I think it's also perfectly acceptable to not expect this navigation to work that way on all themes, as long as there are measures taken to ensure it's visible in nearly any theme (even if it's on top of the content instead of pushing it over). CSS layers should hopefully allow this without z-index chaos.

ckrina credited bnjmnm.

ckrina’s picture

claireristow’s picture

Here is a summary of our Zoom discussion about using the drupal-off-canvas-main-canvas class for the shifting of main page content.

We have decided to move forward with this method. We discussed adding padding to the body element as an alternative but agreed that this would likely cause more headaches than the off-canvas approach. The worst case scenario if a developer removes the off-canvas class would be that the sidebar overlaps with some content when logged in.

We agreed the following should be done to help communicate the importance of keeping the off-canvas class when customizing themes:

  1. We will add a comment in HTML explaining the use of the off-canvas class and what will happen if it is removed (sidebar overlap with main page content).
  2. We will add this as a note in the changelog.
  3. If/when an article is written about changes to the Drupal sidebar, this should be mentioned as something devs should look out for when customizing themes.
claireristow’s picture

Assigned: Unassigned » claireristow
Status: Needs review » Needs work

I am going to work on that remaining bug that @mherchel flagged in comment #6

claireristow’s picture

Assigned: claireristow » Unassigned
Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

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