Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Toggling the toolbar tray can change a table's width, and sticky header calculations rely on this width.
Currently, toggling the tray does not recalculate the width, so things like this can happen:
To reproduce,
1. Go to admin/content, scroll to where the header is sticky.
2. Open the tray.
3. Refresh the page and the tray should still be open.
4. Close the tray and the sticky header will still be at the tray-open settings.
Proposed resolution
tableHeaderResizeHandler should also fire on drupalToolbarTrayChange
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | 3145930-5-89x.patch | 1.17 KB | lauriii |
| #10 | interdiff.txt | 1023 bytes | lauriii |
| #7 | after.png | 618.81 KB | msuthars |
| #7 | before.png | 602.19 KB | msuthars |
| #5 | Screen Shot 2020-06-09 at 7.01.15 AM.png | 376.74 KB | bnjmnm |
Comments
Comment #2
bnjmnmComment #3
msutharsComment #4
msuthars@bnjmnm I reviewed the patch. I applied the patch & clear the cache but the issue is still there.
Comment #5
bnjmnmLooks like the compiled .js file was not included in the patch. Took care of that here.
@msuthars, in future js/css reviews, could your screenshots include evidence that the browser is loading the changes in the patch? I attached an image on how this should look. For JS and CSS issues, it's often not clear if a problem is simply due to an uncleared cache, and that can slow down progress.
Comment #6
msutharsComment #7
msuthars@bnjmnm Thanks for the suggestion. I retested the patch #2 but I have to manually compile that js then it works.
I tested the patch #5 and it is working as expected. No need to manually complie. Moving to RTBC.
Before:
After:
Comment #8
nod_RTBC+1
seems like the tableresponsive script doesn't trigger a columnschange event when showing/hiding columns. Tableheader works because it is itself a responsive table. Out of scope though.
Comment #10
lauriiiAlso tested manually and could reproduce the problem and that it's fixed by this.
Committed ca0af47 and pushed to 9.1.x. Thanks!
This is adding another event to the event listener which doesn't seem disruptive. Leaving as RTBC for perspective from another committer whether this should be backported or not..
Comment #11
alexpott+1 for the backport - I'd go for 8.9.x if this still applies there...
Comment #12
lauriiiOpened follow-up to add test coverage for table header (including this use case) #3155053: Add tests for the table header.
Comment #13
lauriiiRerolled this to 8.9.x
Comment #16
lauriiiBackported to 8.9.x and 9.0.x.