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.Vanilla JS rewrite of tableselect / tableheader
| Comment | File | Size | Author |
|---|---|---|---|
| #30 | table-select-dark-mode-after.png | 606.21 KB | batigolix |
| #30 | table-drag-light-mode-after.png | 286.68 KB | batigolix |
| #30 | table-drag-light-mode-before.png | 256.26 KB | batigolix |
| #30 | table-header-light-mode-after.png | 225.01 KB | batigolix |
| #30 | table-header-light-mode-before.png | 278.62 KB | batigolix |
Issue fork gin-3294539
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #3
saschaeggiComment #4
saschaeggiComment #5
DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commentedFunctionality wise the table header & table select seems to work as before. How can we test the sticky header behaviour? Where is it enabled in core/contrib, or how do we enable it on a custom table?
Comment #6
saschaeggie.g.
/admin/contentComment #7
DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commentedFound an issue: the table header is not aligned anymore with the table columns on
/admin/content. It is when the table header is positioned normally, but it widens once the header detaches and becomes sticky.Comment #8
saschaeggiFixed. I also found another small issue when the initial page state would be scrolled and the sticky header is not showing up.
Can be reviewed again.
Comment #9
DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commentedStill having the same issue as in the screenshot I posted. Tested using Chrome and Orion browsers.
Comment #10
saschaeggiStrange, just tested again with Chrome, FF & Safari, works as expected 🤔
Make sure you have the latest version of this MR for testing.
Comment #11
DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commentedI tried again and I'm 100% sure the latest version of the MR is applied. The pixel value width is being applied to the
table.sticky-header > thead thelements, and the width is being updated on window resize. My guess is that the pixel value is just somehow too big. Some context that might be relevant: I'm using the horizontal toolbar and some table columns are sortable.Comment #12
saschaeggiPushed an update. Still can't reproduce the bug reported by @DieterHolvoet
Comment #13
saschaeggiComment #14
saschaeggiComment #15
saschaeggiComment #16
Kristen PolFixing tag.
Comment #17
Kristen PolWhoops... 2 tags needed fixing... fixing the other one now.
Comment #18
LittleCodingComment #19
saschaeggiComment #20
saschaeggiThanks again for reviewing another one @LittleCodding 💙
This is ready for a re-review ✌️
Comment #21
saschaeggiBack to review 😊
Comment #22
LittleCodingIn the JS file `gin/js/overrides/tableheader.js` The functions/methods 'stickyPosition', 'createStickyHeader', 'syncSelectAll', and 'handleResize' should be in their own name spaced object `Drupal.ginTableHeader`.
Comment #23
saschaeggiThanks again @LittleCoding for taking the time.
Back to review 🏓
Comment #24
saschaeggiComment #25
saschaeggiComment #26
saschaeggiComment #27
saschaeggiComment #28
saschaeggiComment #29
batigolixComment #30
batigolixI tested the patch and it applies well.
I checked 4 situations:
- table header in light mode
- table select in light mode
- table select in dark mode
- table drag in light mode
I did not find any issues.
Comment #32
saschaeggiThanks!
Comment #34
berliner CreditAttribution: berliner commentedAnyone else coming here looking the reason for the sticky header being too wide, as described first in #3294539-7: [JS Refactor] tableselect / tableheader, setting the table to `width: auto;` seems to fix it for me.