Sticky table has problems with responsive column toggle

sanket.tale → made their first commit to this issue’s fork.

Hi, Created MR for the issue please review it. Thanks!
Changing the assignee as I have already worked and created MR.

Disabling the functionality is not a solution. This just hides the visual glitch, but also breaks the functionality of the responsive column toggle.

@hudri is still still a problem on the latest 4.0.x branch?

Yes, I can still see the same issue on my test instance with 4.0.x, commit b95e4e3

Just for reference, this is how it looks on a non-sticky-table:

This example is from a multi-cardinality paragraph field. Both toggles (weight and columns) are rendered in source code, but only the weight toggle is used/visible (the column toggle is hidden, there is no responsive setting for this table)

Hi, I've resolved the sticky table issue using a different approach. Please review the MR !444. Attaching the before and after video clips for reference.

Hi
I've tested MR 444 on Drupal 11.x
MR is applied cleanly...

The Sticky table issue is resolved for responsive views and visually looks good.

Attaching Screen recording for reference
RTBC+1

Keeping in "Needs review" for code verification

Not sure what exactly was tested here, but both MRs actually remove the sticky table headers in general.

@saschaeggi, the issue was that on resolution less than 960px the Show all columns option was not being rendered when using Gin theme. Only few Columns of the table were being rendered and there was overlapping of the same table header within the table. I've fixed the sticky table headers issue and the responsive column toggle option which is being rendered for max-width: 959px. After fixing the issue Show all columns are now being rendered and have the sticky table header also. But noticed an issue for resolution less than 835px on clicking the Show all columns the overflow issue is occurring. This is due to the use of overflow property on the parent of sticky element <div class="gin-table-scroll-wrapper gin-horizontal-scroll-shadow syncscroll" name="gin-sticky-header">. If we fix the overflow issue then the Sticky table header won't work for small devices because CSS property position: sticky won't work if any ancestor of the sticky element has overflow: hidden, overflow: scroll or overflow: auto property. If we fix the issue of overflow then there'll be no sticky header for resolution less than 960px as the position: sticky won't work with overflow-x: auto; overflow-y: hidden; and if we want the sticky table header then there'll be overflow issue on small devices. So, need your feedback regarding this issue.

I've attached the before and after fixes attachments.

Hey everyone,
I have reviewed the Merge Request !444 and here are my findings:

I have observed that the overlapping issue of the scroll wrapper and the hidden 'show all columns' button seems to persists for screensizes below 1319px and not only for below 959px. So after applying the patch , the overlapping issue and hiding of the button only seems to be fixed for screensizes below 959px and not for others.

Another thing, I noticed that after applying this patch , the sticky scroll bar was not working for devices having screensize below 959px.

Attaching screen recording for references. Thanks and regards.

nayana_mvr → changed the visibility of the branch 3455723-override-tableresponsive-js to hidden.

As mentioned in previous comment, the changes in MR444 the sticky scrollbar won't work on devices below screen width 960px as it hides the sticky header using display: none !important. I think the main issue is that the tableresponsive-toggle-columns element is getting appended before the main table instead of the sticky table header of Gin as it is following a different table view template. So I have created a new patch where I override the core tableresponsive.js as per the Gin view table template and now the table sticky header and Show all columns button is working as expected. Please find the attached patch as I'm unable to create MR.

Sorry, there is a minor update in the style. Please see the updated patch.

nayana_mvr → changed the visibility of the branch 3455723-override-tableresponsive-js to active.

Hi @Hritick, I've reverified the changes made on MR !444 and the Show all columns/Hide lower priority columns option will only display upto screen resolution 959px. The issue of overlapping persists only upto resolution 959px and I tried reproducing it for 1319px but didn't get the overlap issue as shown in the provided video. I've attached the video clip for your reference. As mentioned in the comment #15 🐛 Sticky table has problems with responsive column toggle Needs review the sticky table header will work only upto screen resolution 960px if the overflow issue fix.

@nayana_mvr, I've applied the provided patch 3455723-tableresponsive-new.patch → and it did not fix the issue at all. After patch Show all columns/Hide lower priority columns is now displaying for resolution greater than 959px if we click on the Show all columns/Hide lower priority columns button. And the sticky header behavior is being maintained after the patch but it breaking the table design as shown in the provided video. The table header is now shown wrong for the table data if Show all columns is visible. I've attached all the screenshots and video clips for your reference.

As I'm not encountering the issue mentioned in #16 🐛 Sticky table has problems with responsive column toggle Needs review leaving the status Needs review. Anyone can review the MR and provide the steps to reproduce issue mentioned in #16.

Hi @Tirupati_Singh, thank you for pointing out the issues. There were some more code changes required while overriding tableresponsive.js. I have fixed the mentioned issues. But I'm not sure if this is the correct solution approach for fixing this ticket. Please find the new patch attached. Also attaching the screen recordings for reference.

Note:- After switching to 959px screen and if the user clicks on the Show all columns button and switch back to resolution greater than 959px, then Hide lower priority columns button will be still displayed. But once you click on the Hide lower priority columns , the button will get removed in that resolution. This is a default behaviour and will find in core theme also. You can find the comments related to this in the tableresponsive.js code itself.

nayana_mvr → changed the visibility of the branch 3455723-override-tableresponsive-js to hidden.

Also attaching the screen recording with the latest changes for reference.

I've tested the MR!444...

Its fixed the sticky header issue for the devices under 959px.

Attaching screen recording link: https://www.awesomescreenshot.com/video/31139234?key=a191b06395bb096ae03...

Hence moving it to RTBC.

The MR needs a rebase 👀