Offset the html anchor to adjust for Toolbar

I've added a proposed solution

Sounds like a duplicate of #1870006: HTML5 validation with table sticky header is misaligned over the toolbar, FYI #1440628: [Meta] javascript toolbar/tableheader with url fragment mess

Leaving open because the solution looks really simple, let's see how it goes.

If it's not a CSS fix, definitely not a novice issue.

Matthew Lambert and me are handling this Dublin2016 (:

@Ehud can you assign this to you

Looks like the proposed solution doesn't work. I applied it and played with it.. So probably javascript will do it.

I think @joelpittet's solution will work with some tweaking. Something like this:

.toolbar-fixed :target:before {
  content: "";
  display: block;
  height: 79px; /* height given to toolbar */
  margin-top: -79px;
}
.toolbar-vertical :target:before {
  height: 39px; /* height given to toolbar */
  margin-top: -39px;
}

I don't see how we get away *without* using some specificity as we can't just simply use :target:before as it will affect users without access to the toolbar.

I'd submit a patch, but someone will need to tell me where this code should go :).

Making this a new comment to see the diff, but I like this better as it accounts for:

1. Toolbar tray closed
2. Toolbar open horizontally
3. Toolbar open vertically

.toolbar-fixed :target:before {
  content: "";
  display: block;
  height: 39px; /* Amount of padding toolbar gives to body */
  margin-top: -39px;
}
.toolbar-horizontal.toolbar-tray-open :target:before {
  height: 79px; /* Amount of padding toolbar gives to body when the tray is open horizontally */
  margin-top: -79px;
}

@chrisrockwell totally agree, it needs to target the toolbar! Could you make a patch for that and we can find someone to test it out?

Attaching #12 as a patch. Do we need two separate patches for Stable and core/modules/toolbar? Or - maybe we can remove toolbar.module.css from Stable since it appears to be an exact copy?

I'm still unsure how to deal with bugs in CSS and Stable... it would be nice to have a clear way to make that call from the Stable subsystem maintainers. For now I think having them together is good until deemed appropriate to split.

Marking for manual testing for someone to review, test if it works and mark this RTBC if so. Thanks for the patch @chrisrockwell

Unless there is an argument beyond "don't touch Stable" I think we should be able to do this. It is a bug fix applied to a sudo selector. It only affects the toolbar, which is an administrative element, and doesn't affect its actual internal styling.

Manual testing for sure, and test on a theme other than Seven. I would test with Stable, Stark (which doesnt use Stable), and another.

maybe we can remove toolbar.module.css from Stable since it appears to be an exact copy?

No, that is its purpose, to be a copy. If we have to, we can make these changes to the core toolbar CSS and not touch the Stable CSS. The easy fix then becomes to remove the Stable CSS in your theme and reimplement the core toolbar one. We could actually do that with Seven.

My comment is speculative, since I haven't had the opportunity to test the actual patch.

I used to run a JavaScript solution for this in D7, since the height of the toolbar could vary as it wrapped menu items to the next line as needed. The D8 toolbar is a fixed height, since you're using pixel units? What if someone changes the font size?

@kpaxman, you are right, we should at least put rem values in there like we do with other pixel values.

We could use JS and listen to the same events and just read the body padding-top values. Was hoping for CSS for a the simplicity of it.
The body padding is set here: Drupal.toolbar.BodyVisualView

Or maybe instead of listening to the events we could listen to body changes with mutation observers?
http://caniuse.com/#feat=mutationobserver

Here's a prototype of something that could work:
https://jsfiddle.net/91w05w58/1/

I noticed we've been ignoring IE9 and IE10 recently to some extent but there are polyfills for this if need be.

I've been considering this as a JS only solution (which is reasonable since it's the toolbar). What about this patch that adds a style tag and just lets the toolbar manage the height and negative margin? I'm assuming this could be moved into Seven's js if it cant' go into toolbar.

From my testing, this addresses @kpaxman's concern.

I've doing a lot of research into this, and discussing with @joelpittet, and I'm coming to the conclusion that, IMHO, this isn't the responsibility of toolbar. What about adding some css to stable and classy?

Bootstrap accounts for the toolbar by hard-coding the values:

body.toolbar-fixed {

  // Fix z-index.
  .toolbar-oriented .toolbar-bar {
    z-index: ($zindex-navbar-fixed + 1);
  }

  .navbar-fixed-top {
    top: 39px;
  }

  // Horizontal.
  &.toolbar-horizontal.toolbar-tray-open .navbar-fixed-top {
    top: 79px;
  }

I propose this patch that works very well, which allows to scroll to the correct element required that contains the error generated when not filled

I propose this patch that works very well, which allows to scroll to the correct element required that contains the error generated when not filled

I just hit this after finding #546126: Toolbar interferes with anchor links... It looks like things have come full circle... :)

After a bit of struggle managed to get the CSS in #12 working. (#21 didn't seem to work for me because my subtheme SCSS didn't have access to $zindex-navbar-fixed. Any ideas on how to get around this?)

I do have a word of warning: #12 fails (in Firefox and Chrome) if the targeted div has a border of any kind. In Bootstrap, divs with class "panel" have a 1px solid transparent border.

It's too late on Friday for me to have a theory as to why this might be. I can't see why it would fundamentally alter how the ":before" CSS is functioning.

It might make sense to use a proper name attribute on an orphaned anchor tag, rather than trying to reference the div's by their id attribute.

The patch in #14 works for me in Firefox and Chrome and also with changing the font-size. I think a CSS solution is preferable to a JS solution.

This is a duplicated issue : https://www.drupal.org/node/1870006

I'm afraid I need to re-open this issue, since #1870006: HTML5 validation with table sticky header is misaligned over the toolbar only partially fixed the issue.
If user with toolbar clicks the anchor link, it is positioned correctly, but if the anchor is reached on full page load (e.g. someone sends the link), then the Toolbar is on top of the anchor.

This was a bugsmash target issue today. smustgrave was not able to reproduce the problem and suggested this is outdated. larowlan said we now have the offset library and agreed.

Therefor, I am closing this as outdated.