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... 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 🐛 Sticky table header is not sticky if --drupal-displace-offset-top is not defined Fixed 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)

@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.

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

nice thank you! I've just manually retested with 🐛 Sticky table header is not sticky if --drupal-displace-offset-top is not defined Fixed and only MR200 for this issue applied (tested also with both MRs applied before the Claro issue went in) on a fresh install of 11.x-dev, the sticky headers both on /admin/content and /admin/people/permissions are correctly shown without any offset sticking to the top of the page. I've also tested with MR201, visually the same effect, the sticky table header is shown without any offset.

Yes but resolving table problems should not be 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 can 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 theme which is active.

@Tirupati_Singh we had same disqussion in
https://www.drupal.org/project/navigation/issues/3402592 ✨ Reset theme css. Fixed

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 🐛 Sticky table header is not sticky if --drupal-displace-offset-top is not defined Fixed 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 we are really have top bar rendered (this is what was fixed in 200 mr)

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 201 MR in current 11.x core context?

ckrina → changed the visibility of the branch 3432222--the-sticky-header-has-an-effect to hidden.

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.

ckrina → closed merge request !200

ckrina → reopened merge request !200

joaopauloc.dev → made their first commit to this issue’s fork.

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.

ckrina → closed merge request !200

ckrina → reopened merge request !200

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?

i've applied the MR200 to the navigation module with latest changes in including the drawer issue, and in my case sticky is working

ckrina → closed merge request !200

ckrina → reopened merge request !200

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.

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

ckrina → closed merge request !201

Let's test it with drupal 11.x because related fix in Claro theme was provided only for 11.x https://git.drupalcode.org/project/drupal/-/blob/11.x/core/themes/claro/...

For 10.x sticky table header doesn't work (due to missing fix in Claro).

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.

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