-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Make table header sticky #786
Conversation
0d46d3a
to
051c362
Compare
Looks good. I did not look to deep into the code. But would this also work if we have multiple tables on a page? (While adjusting the ncTable component, I am struggling with the stickiness in general. Or was that the trigger for this fix? 🙃) |
No it was not directly related. I think this should also work with multiple tables on the same page where sticky headers from lower tables should stack on top of the upper header. Though would not exclude that we need some additional special handling for that (especially since the tables might have different width) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reasonable
Can we merge this? @juliushaertl |
I still need to figure out how to do the scrolling with the link previews 🙈 Will try to get that done the next days |
051c362
to
ed5d21a
Compare
Signed-off-by: Julius Härtl <jus@bitgrid.net>
ed5d21a
to
499ef92
Compare
Ready from my side, maybe you can have another review @enjeck Screen recordings added to the pull request description. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool 😎
Awesome job, I was looking exactly at this issue, thanks for solving it ! |
Slightly different approach for #342 to contribute to #631 making the table column header sticky.
Ideally we would have a combination of absolute positioning and stickyness to let the sticky container only span the outer width of the scroll container, but that seems to be impossible so I went for a resize observer that pins the width of the sticky table header and table options through a css variable which turned out to work reasonably well.
@datenangebot I don't remember exactly if the points in the old pr are still valid, but considering the refactor would still be valid, this seems like a good intermediate solution for browsing larger tables.
Also fixes #474
ToDo