- Issue created by @ressa
- 🇫🇷France dydave
Super good catch @ressa! 🥳
Thanks a lot for the very clear ticket:
No problem, we're going to fix this: I will take a look a little bit later today, when I have a bit more time 👌
Otherwise, I have the feeling this release went quite well overall 👍
Apart from users having issues running DB updates, we didn't get anything else really relevant to this upgrade.
I'll take also a look later in the day at 🐛 Upgrade version 3.5.3 to 3.6 crashes site Active .
Thanks so much, once again, for all your help answering issues and helping users updating their sites correctly! 😅
- 🇩🇰Denmark ressa Copenhagen
Sounds fantastic, thanks as always for a fast and positive response @dydave! And this is such a small detail, that it can easily be put on the 3.7 Meta list, and handled when it's convenient, so I have added it there.
And yes, it does seem like all the thorough and diligent testing (and let's not forget you writing tests in the first place!) is paying off, and the only bump so far for the users, were the missing database update, or in combination with other contrib modules -- it's pretty impressive 🙂
- 🇩🇰Denmark ressa Copenhagen
Ps. I was translating Admin Toolbar, when I discovered a detail about strings, where "toolbar" and "sticky" is reversed in one place, and shown as a translatable string:
https://localize.drupal.org/translate/languages/da/translate?project=adm...
$ grep -iR "toolbar sticky" . ./js/admin_toolbar.sticky_behavior.js: * Admin Toolbar sticky behavior JS functions. ./src/Form/AdminToolbarSettingsForm.php: '#title' => $this->t('Toolbar sticky behavior'), ./src/Form/AdminToolbarSettingsForm.php: '#prefix' => $this->t("By default, the Admin Toolbar sticky behavior is <em>enabled</em> so it stays at the top of the browser window when scrolling up or down.<br/>Select <em>Disabled</em> to disable Admin Toolbar's sticky behavior so it stays at the top of the page when scrolling up/down and does not follow the browser window."), ./admin_toolbar.libraries.yml:# Disable toolbar sticky behavior. $ grep -iR "sticky toolbar" . ./config/schema/admin_toolbar.schema.yml: label: 'Sticky toolbar behavior'
Suggestion: Use "Toolbar sticky behavior" in
admin_toolbar.schema.yml
.It's such a small thing, so maybe fixing it here as well is easiest? 🙂
- 🇫🇷France dydave
Totally! Thanks a lot @ressa.
I'm adding here as well the change of wording in the settings form we couldn't get in the release:
#3304810-72: Toggle away Admin Toolbar completely → - 🇺🇸United States erutan
I'm noticing something similar with views tables that have 'sticky headers' enabled. Inspecting it shows it seems to have the correct offset in and of itself:
table.sticky-header thead { position: sticky; z-index: 500; top: var(--drupal-displace-offset-top, 0); }
It appears properly aligned below the menu bar when it exists, but doesn't pop back up to the top when the admin menu hides itself, which is a bit odd as it just floats in the middle of the table. It reseats itself to the top of the table properly when scrolling up. I can create a new issue, but this sounds like it has the same root cause.
- 🇮🇳India Tirupati_Singh
Hi @dydave and @ressa, I've followed the steps to replicate the issue and can confirm that the issue persists. When choosing the option "Disabled, show on scroll-up" under Toolbar sticky behavior setting of Admin Toolbar Settings, on scrolling down and up the attribute "--drupal-displace-offset-top" is not getting restored respectively, which is triggering the issue for all the tables that have "Enable Drupal style "sticky" table headers" property enabled as @erutan commented on #6. I'm attaching the screenshot for reference.
- 🇮🇳India Tirupati_Singh
My bad, forgot to attach the attachments. Attaching here. I'm also looking into the issue.
- Merge request !152Issue #3526123: Fixed sticky table header issue when toolbar sticky disable,... → (Open) created by Tirupati_Singh
- 🇮🇳India abhishek@kumar
CSS-based Solution
[data-toolbar="disabled"].toolbar-fixed { --drupal-displace-offset-top: 0px; } [data-toolbar="disabled"].toolbar-fixed.toolbar-tray-open { --drupal-displace-offset-top: 79px; /* Adjust as needed */ }
- 🇮🇳India Tirupati_Singh
@dydave, I've fixed the sticky table header design issue that occurred when the option "Disabled, show on scroll-up" is selected. Please review the MR and let me know if any further changes are needed. I've attached screenshots of the fix for your reference.
Thanks!
- 🇮🇳India Tirupati_Singh
@abhishek@kumar, I've tried the CSS shared in comment #10, but it didn't work as expected. The attribute [data-toolbar="disabled"] is not present in the DOM during testing on Drupal 10.4.7.
Let me know if there's an alternate approach or if this behavior is version-specific.
- 🇺🇸United States erutan
The CSS in #10 didn't fix the issue in either my admin or default/site theme. Tested in Safari & Chrome.
The patch from the MR by @tirupati_singh works for me, thanks! It doesn't seem like it'd cause any other issues, the existing disabled and enabled for admin toolbar sticky still work as expected.
Waiting for a 3.7 minor release doesn't make sense as this is a pretty obvious/annoying visual bug IMO. :)
- 🇺🇸United States justcaldwell Austin, Texas
Setting back to Needs work
As indicated in comments #11 and #12, MR 152 only address the issue when using the "Disabled, show on scroll-up" setting. The problem persists if you're using "Enabled" or "Disabled"
- 🇺🇸United States justcaldwell Austin, Texas
We need to toggle the
data-offset-top
attribute on the toolbar-bar and any active toolbar-tray so Drupal.displace() will recalculate offsets → correctly. Updated the MR to do that.This is working for me if toolbar sticky behavior is set to "Enable" or "Disabled, show on scroll-up". It does not fix the "Disabled" case yet.
I think we probably need a new js behavior the "Disabled" case that also unsets
data-offset-top
attributes. So, still "Needs work" but testing/review of the latest changes appreciated!