The sticky header has an offset

Confirmed described problem is really exist.

Because here https://git.drupalcode.org/project/navigation/-/blob/1.x/templates/top-b... we have "data-offset-top" attribute added no matter if top bar is rendering local_tasks or not.

And no matter if this block is hidden by some CSS rules or not -> core's displace() is still measuring height of this block https://git.drupalcode.org/project/drupal/-/blob/11.x/core/misc/displace... (you can google how jquery outerHeight() is working) and creating --drupal-displace-offset-top CSS property in style attribute of html tag. That's the reason why table header have top offset.

This issue requires 2 fixes:
1. first fix is already provided in MR - now we are not rendering "data-offset-top" attribute if no local_tasks in render.
2. and another fix should be done in core. I have created new ticket in core for Claro theme https://www.drupal.org/project/drupal/issues/3432298 which i have fixed already . Reason is simple - once i have kicked "data-offset-top" attribute - there is no anymore css property "--drupal-displace-offset-top" defined, which breaks stickiness of table header (read description of core task i have created to understand better)

Important note for reviewers
You have to apply patch from https://www.drupal.org/project/drupal/issues/3432298 also

@kostyashupenko, fixed the sticky header issue. Please review the MR! 201, attaching a screenshot for reference.
I've reviewed the MR! 200 the issue still exits. So, I've created new MR! 201.

@Tirupati_Singh it is usually bad practice to use !important in css.

Lot of articles about it. This is first found https://www.linkedin.com/pulse/using-important-css-generally-considered-...

@finnsky, I've removed the !important css property.

Yes but resolving table problems should not be done in the scope of navigation module

@kostyashupenko,

Yes but resolving table problems should not be done in the scope of navigation module

The changes made in the CSS property should be done in this module because if the Navigation module is enabled then only the top Navigation toolbar will be hidden and the sticky header issue will be there. If the changes done in CSS property should not be in the scope of Navigation module then other issue will be there in the active theme.

@Tirupati_Singh we had same disqussion in
https://www.drupal.org/project/navigation/issues/3402592

Module styles should only describe the module.
In other matters, such as the positioning of global elements on the page, we are guided by how the core works.
This is drupalDisplace.

@Tirupati_Singh The displace functional described in core here https://git.drupalcode.org/project/drupal/-/blob/11.x/core/misc/displace... so no matter what we are building - module or theme, we should respect core displace functionality.

This task have 2 problems:
1. Table header in Claro theme (see https://www.drupal.org/project/drupal/issues/3432298 and btw this task is already fixed)
2. "data-offset-top" html attribute in Navigation module - was added always, but instead should be added when top bar is really rendered (this is what was fixed in 200 mr)

Please re-test it, because you said that 200 mr is not working in #7, but i think it should work

Hi
I've tested MR !201 on Drupal 10.2
The patch was applied successfully...

Adding files for the references

@Kanchan Bhogade could you please test 200 MR in current 11.x core context?

Hiding MR 201 as the approach to follow is the one in MR 200, as explained in #14and #16.
Thanks for the attempt with the alternative approach in 201, but the direction is defined in 200.

Hi @finnsky,
I've tested MR !200 on Drupal 11.x
ThePatch applied successfully...

Test Result:
The header is sticky at the top of the table

adding files for the reference

Sorry @kostyashupenko but I just tested 200 (FF and Chrome) and the header of the table doesn't stay sticky, so moving it back to NW.

Hi folks.
Nice work here, this module will increase a lot the usability of Drupal admin UI.
I found this issue because @rkoller mentioned this one in another issue.
Fixed the typo and works fine. See the image below.

Thanks for the catch @joaopauloc.dev! Then I guess this MR only needs to be updated with the latest changes to be tested&merged. Moving to Needs Work to give some visibility to it.

merge requested updated with the latest changes in 1.x branch.

Thanks again @joaopauloc.dev! I can't see it working as in your GIF now. I wonder if it has something to do with the new drawer that we just merged?

It's working for me too.

Try to clear the browser cache since we have CSS files changed.

Sorry I messed up with the commits 🤦‍♀️

Anyway, I checked it locally too with FF and Chrome and I still can't see it working. I've pushed the changes from 1.x (in the wrong way), but the Tugboat link is working now. Do you see it working there? Or I am missing something?

The changes of the branch didn't work for me, so I overridden the CSS value to use top: 0 instead. This is working for Drupal 10.2.4. Wrong patch, sorry, this is to fix Claro theme.

@ckrina the current link on the Tugboat at the moment that I'm accessing is redirecting to another website.

Let's test it with Drupal starting from version 10.2.x-dev or above https://git.drupalcode.org/project/drupal/-/blob/10.2.x/core/themes/clar...

On <= 10.2.4 MR in this ticket will not work with Claro admin theme

Here is screenshot from 10.3.0-dev

Good catch @kostyashupenko! Now we know why we were seeing different things :)

I've tested it locally with 10.3 and works, so setting the auto-merge for when the checks are ready. Thanks!

Added credits.