Node preview bar should re-calculate BODY padding-top, otherwise user menu (top of page) is obscured

We probably want the preview bar to push everything down? Perhaps we even want it to sit above the toolbar? Not sure.

Yes pushing it down would probably be ideal. A bit tricky with position fixed items in the theme (aka sticky headers) although that would be a problem regardless.

Not sure what pattern is being used for the navbar but maybe they can work the same and together?

We should be able to hook into #1847084: Measure/track displacing elements better + provide change events for them (fix overlay + toolbar)

This is node specific.

Double post.

#2 Yes, for those kind of changes we have Drupal.displace (in displace.js).

I'm wondering if we should even fix this, if a user clicks on those links they loose their changes to the node. Is this a use case we want to support?

I think its very annoying especially on a smaller screen if the admin menu doesn't push the content down. It might not be that big problem with bartik but with your custom theme it might be PITA.

This is just the node preview bar though?

There is no patch attached to this issue to work on, so I believe this should be on 'Active'.

Marked #2550691: Node preview bar occludes page header, should use Drupal.displace() JS API instead of CSS as duplicate to this, but it has some ideas on Proposed resolution - use Drupal.displace(), which was designed for this.

I attempted to fix this issue, but I believe I'm missing something.

Patch attached does displace the bar, but for some reason its not displacing the page. Using Drupal.displace wrong?

I will look into this issue today. The problem seems to be that Drupal.displace does not deal correctly with multiple elements that have data-offset-top set.

Setting data-offset-top on .node-preview-container does not solve this issue as you can see in the screenshot from comment #14.

The problem is that Drupal.displace assumes that every displacing element is positioned correctly while .node-preview-container has neither top or margin-top set. The toolbar module does it's own calculations and sets margin-top in ToolbarVisualView.js line 255.

Possible solution. The Node Preview toolbar could do it's calculations, check the orientation of the toolbar, get the height of the toolbar but this does not seem to be generic enough aproach. What if the user installs another toolbar module?

Another solution would be not to use Drupal.displace and instead set padding-top on the body element to whatever the value is now + the height of .node-preview-container.

Yeah, it feels like part of the problem is that the node preview container is injected into the page as a separate component to the toolbar and then styling to look like it's part of the toolbar UI.

I think this would automatically work if we moved this node-preview-container to the toolbar module, rename it to something generic like toolbar-information-container and then allow the node module to inject into it somewhow. How does that sound?

Not sure about the best solution. Giving a chance to others to take over this task.

If we added it to toolbar, and someone turned toolbar off, then what?

@kylebrowning Good point... what happens when you disable the toolbar now?

I tried with the patch applied and it still doesn't fix the issue:

Displace.js doesn't seem to be running. Is there something else we need to pass into Drupal.settings?

I suggest to add class to body on preview page and to add padding-top that depends on that class. It doesn't depend on Toolbar module.

and what's about "displace"?

@andypost As mentioned in #16 Drupal.displace should work fine if the preview bar would be positioned using CSS top and margin-top values the same as the toolbar does. The preview bar currently has no top nor margin-top values set, this makes Drupal.displace push it down every time it runs.

This issue https://www.drupal.org/node/1847084 led to the creation of Drupal.displace. If you want to get an idea how the code works see bottom part of comment #69.

The problem is how do we get a correct top and margin-top values taking into account that the toolbar changes it's layout, that the toolbar might be disabled or there might be other elements that are displacing content. It seems Drupal.displace could be extended to provide some sort of global registry for displacing elements. A similar idea was proposed in the same aforementioned discussion in #66.

When the layout changes because of the toolbar or something else having to do with displace we have an event firing, use that: http://read.theodoreb.net/drupal-jsapi/global.html#event:drupalViewportO...

Drupal.displace already does too much (it shouldn't have to figure out element size automatically) so I'm not willing to expand on that, especially now.

Ok, we cannot expand Drupal.displace to register all displacing elements. This means that every fixed element must determine it's own correct position. The preview bar therefore must know what elements are already position: fixed, determine their height, set it's own top and margin-top value and update those when layout changes. This sounds tricky.

A better way to go about this could be to have all displacing elements in one container. That container would be positioned with position: fixed; top: 0;. Then it's children would not have to be positioned fixed and no JS height calculations would have to be made.

Lewis already mentioned a similar idea in #17.

Of course, we can't expect every module to place it's displacing elements markup into one template. Modules could mark elements as displacing the same way it is done now with the data-offset-top attribute and instead of passing an offset value in pixels they would pass a weight value for ordering. This could look something like this: data-offset-top="-20". Than a script would pick up all displacing elements, sort them by weight and place them into the one single fixed positioned container.

Drupal.displace was built to be a convenient tool for a single displacing element that can be aligned to any edge of the viewport. The real world use case is rather that we have multiple items. Drupal.displace is good at what it does but it just doesn't solve the problems we have with multiple items.

These changes would introduce a new feature and most likely will not cut it into 8.0. Possible solutions or rather workarounds for 8.0 could be:

1. Leave it as it is. For a high percentage of users it shold not be critical that about 30px of the top of the page are hidden. It is a preview after all.
2. Change the theming or layout of the preview bar. Make it semi-transparent, reduce height, position absolutely etc.
3. Change the UI of the preview bar. For instance make it a button that expands on tap to show all options.

@alexpott, @catch, @cilefen, @effulgentsia, and I discussed this issue this morning while triaging issues with the entity and field system maintainers. We agreed that this does not meet the criteria for a major bug. While there is definitely a visual bug, as @bruvers points out, the impact is not that serious for most sites and content creators.

Thanks for working on this issue!

1. the Preview bar is position: fixed;
2. the padding-top is tracking & applying via Toolbar.
3. Preview bar needs to detect the Toolbar status. If Toolbar is found, it should delay the BODY "padding-top' re-calculation after Toolbar.

There's an OutsideIn module, I think we should keep the visuals consistency of these top bars.

Rerolled patch failed to apply on 8.9.x

Rerolled patch for 9.3.x and added a removeClass() so we don’t leave cruft on detach.

I'm not sure if this is still needed. Tested in both Umami and Olivero and it looks good to me

Closing out for now as it's been 3 months since #47 and can no longer be reproduced, it seems likely this was an issue with Bartik.

Adding credit for all the good work that was done here.

Saving triage credits from #33 and for @acbramley, and for various issue reviewers and contributors who worked on the Bartik bug.

I guess "outdated" is better, since it was reproducible previously.