Table header gets overflow on the permissions page (/admin/people/permissions) while there is multiple users exists.
steps to produce :
1. Create multiple user role from here (admin/people/roles)
2. Go to url path (/admin/people/permissions)
Here you can see the bug.

CommentFileSizeAuthor
#85 ksnip_20240604-200922.png194.42 KBdavid.qdoscc
#75 gin-table-header-sticky--3447517-1.patch732 bytesambot112
#71 Permissions_Test.png44.35 KBrobcarr
#69 Screenshot 2024-05-01 at 09.18.13.png111.64 KBbram.velthoven
#69 Screenshot 2024-05-01 at 09.14.23.png142.42 KBbram.velthoven
#67 permissions-2-headers.jpg53.67 KBrobcarr
#63 gin-roles-horizontal-scroll-relative-fix-2.png697.43 KBhebl
#62 gin-roles-horizontal-scroll.mp43.18 MBhebl
#57 whitespace tugboat.png276.24 KBarantxio
#56 cramped table-layout.png167.51 KBarantxio
#56 whitespace issue.png168.6 KBarantxio
#44 After_patch_42.mp4726.72 KBdjsagar
#43 Before_patch_42.mp42.84 MBdjsagar
#43 Before_patch_42.mp42.84 MBdjsagar
#42 table-header-patch.patch1.77 KBfrancois-p-peloquin
#39 gin-permission-header.png100.61 KBswatidhurandhar
#38 gin-3293369-38.patch2.5 KBlamp5
#36 CleanShot 2023-05-11 at 20.26.19@2x.png143.9 KBdieterholvoet
#34 CleanShot 2023-05-11 at 18.48.50@2x.png314.61 KBdieterholvoet
#32 CleanShot 2023-05-09 at 17.25.17.png36.22 KBdieterholvoet
#26 CleanShot 2023-05-02 at 11.53.27.png97.11 KBdieterholvoet
#19 CleanShot 2023-04-11 at 14.11.02@2x.png261.25 KBdieterholvoet
#15 Screenshot 2023-03-28 at 10.35.51 AM.png212.41 KBsantosh_verma
#15 Screenshot 2023-03-28 at 10.42.53 AM.png104.47 KBsantosh_verma
#4 table_header_overflow_issue-3293369-4-image.png119.11 KBshyam_bhatt
#15 Screenshot 2023-03-28 at 10.24.30 AM.png117.76 KBsantosh_verma
#4 table_header_overflow_issue-3293369-4.patch1.01 KBshyam_bhatt
#11 3293369-11.patch739 bytesVinayak.Ambig
Screenshot 2022-06-29 at 7.08.17 PM.png150.39 KBtarun chauhan
#11 3293369-11-after-patch.mov25.91 MBVinayak.Ambig

Issue fork gin-3293369

Command icon 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:

Comments

Tarun Chauhan created an issue. See original summary.

tarun chauhan’s picture

Title: overflow issue for multiple user role on permissions page. » Table header overflow issue for multiple user role on permissions page.
shyam_bhatt’s picture

Assigned: Unassigned » shyam_bhatt
shyam_bhatt’s picture

I have fixed this issue by reducing the font size if the table column is more than 10. Please check the patch.Table header overflow issue fix

shyam_bhatt’s picture

Assigned: shyam_bhatt » Unassigned
Status: Active » Needs review
kmonty’s picture

I've also encountered this issue, but I don't think this patch is the correct approach. For example, I have a site with 25 roles, so this patch still has the issue.

I'd suggest the fix is to ensure the Table Select sticky header continues to horizontally scroll after you've vertically scrolled down the page.

amin.ankit’s picture

Assigned: Unassigned » amin.ankit
saschaeggi’s picture

Status: Needs review » Needs work
tea.time’s picture

This seems related?

abelass’s picture

Vinayak.Ambig’s picture

StatusFileSize
new739 bytes
new25.91 MB

I have fixed this issue by adding overflow-x auto to table parent div wrapper. Please check the patch. Also attached video.

Vinayak.Ambig’s picture

Assigned: amin.ankit » Unassigned
Vinayak.Ambig’s picture

Status: Needs work » Needs review
santosh_verma’s picture

Assigned: Unassigned » santosh_verma
santosh_verma’s picture

I have tested #11,
getting warnings "warning: 2 lines add whitespace errors."
warning

Issue is fixed after applying patch

before
before

after
after

santosh_verma’s picture

Status: Needs review » Reviewed & tested by the community

saschaeggi’s picture

Status: Reviewed & tested by the community » Needs review

I've opened MR !244 for a more elegant solution.
Please review

Cheers!

dieterholvoet’s picture

Status: Needs review » Needs work
StatusFileSize
new261.25 KB

This doesn't quite fix it yet. It makes the table horizontally scrollable, but the table header isn't right yet. It creates a duplicate sticky header which still overflows the screen, without removing the old header.

Screenshot

saschaeggi’s picture

Status: Needs work » Needs review

@DieterHolvoet I'm moving this back to needs review as the MR fixes the described issue here. The permissions page does not have a sticky header btw. Are you using maybe a module which alters this page? Let's keep them separate. Free to open an issue for that module in combination with the permission page.

dieterholvoet’s picture

I can reproduce it on another project with this patch applied as well. No modules are altering the permissions page. No other Gin patches except for this one, applied to dev-3.x. The duplicate header only happens if the permissions table has horizontal overflow, if not it works fine (but it's sticky either way).

dieterholvoet’s picture

Something else: $variables['attributes']['id'] doesn't seem to always be set. On admin/config/development/mail_safety using the Mail Safety module it's not set.

You should probably also remove the var_dump().

sakthi_dev made their first commit to this issue’s fork.

sakthi_dev’s picture

Fixed on scroll header sticky. Please review.

saschaeggi’s picture

LGTM, just needs a community review

dieterholvoet’s picture

Status: Needs review » Needs work
StatusFileSize
new97.11 KB

The issues as described in #19 are still present.

Screenshot

lokesh jamadar’s picture

Assigned: Unassigned » lokesh jamadar
lokesh jamadar’s picture

Assigned: lokesh jamadar » Unassigned
saschaeggi’s picture

@DieterHolvoet sorry for the issue hopping but can you test the MR in #3344925: Sticky header for non views tables? Will this address your issue?

dieterholvoet’s picture

No, after applying that MR to a project no sticky header appears anymore on the permissions page. I also can't apply both that MR and the one here since they give conflicts.

saschaeggi’s picture

@DieterHolvoet that is strange, the other MR enables sticky headers. Did you clear the cache? As it has a template change it most likely won't work without a CR.

dieterholvoet’s picture

StatusFileSize
new36.22 KB

Yes I did! I added a console.log to the JS to make sure the new one is picked up and it is, 'test' is logged exactly once.

Screenshot

The new .gin-table-scroll-wrapper element is also present.

saschaeggi’s picture

@DieterHolvoet I pushed a fix, as there was an issue in Chrome and a bug in FF. Can you try it again? Thanks!

dieterholvoet’s picture

StatusFileSize
new314.61 KB

Now it works, except if the table header overflows the page horizontally. In that case the header appears sticky immediately and never disappears:

Screenshot

saschaeggi’s picture

except if the table header overflows the page horizontally

Which this issue is actually for, so we're good to go on the other one I guess 😉

Also does this overflow issue happen in Claro, too?

dieterholvoet’s picture

StatusFileSize
new143.9 KB

On Claro the page overflows horizontally, but both the normal and sticky headers are in the correct position when scrolling horizontally:

Screenshot

paul121’s picture

Experiencing the same when there is overflow horizontally. Happy to test when something is ready!

lamp5’s picture

Status: Needs work » Needs review
StatusFileSize
new2.5 KB
swatidhurandhar’s picture

StatusFileSize
new100.61 KB

Hi,
I applied patch #38, it makes the table horizontally scrollable and all roles are visible. But once we scroll vertically, sticky header gets attached to table which doesn't show all user roles. It also creates extra space on right side of the page.
Thanks

phjou’s picture

Unfortunately, I couldn't apply the patch on latest RC (with patch of issue below) or the dev branch.
It might be due to this ticket that has been merged and also affect sticky tables: #3386007: Table sticky header appears when table not in viewport

saschaeggi’s picture

Status: Needs review » Needs work
francois-p-peloquin’s picture

StatusFileSize
new1.77 KB

I've created a simple patch that listens to the scroll position of the table wrapper and scrolls the sticky table headers over proportionally.

djsagar’s picture

StatusFileSize
new2.84 MB
new2.84 MB

@francois-p-peloquin I have checked the #42, with gin as admin theme.

Steps to reproduce the issue:

1. After user logged in, go to People > Roles
2. Add multiple roles
3. Go to People > Permissions

the patch #42 applied successfully but there is an issue which need to be fixed this is same as claro.

djsagar’s picture

StatusFileSize
new726.72 KB
arantxio’s picture

So when using 10.2 we also got introduced to a new issue with the header, due to the change from #3362276: Use position: sticky for views sticky table header, we now use position: sticky on the header instead of creating a separate table with the header inside of it.

This causes the header to not stick to the top anymore, this is due to the 'overflow: auto' we use on the "gin-table-scroll-wrapper" class.

Removing the overflow: auto resolves the issue of it not sticking to the top anymore, but obviously this makes the permissions go out of bounds of the wrapper that we are using, which creates another problem on its own.

Due to the change from #3362276: Use position: sticky for views sticky table header we now have a 'position-sticky' class instead of the 'sticky-enabled' class, which also needs some rewriting I suspect.

I don't know if its better to put this into another issue or change up this issue a little bit to fix both of the issues that now revolve around the header.

anybody’s picture

Indeed I can confirm #45! Thanks for the quickfix!

TBH without the "overflow: auto;" (on .gin-table-scroll-wrapper) it behaves 'more' correctly to me. The column labels are always positioned correctly then and the table header is sticky.

guilhom’s picture

I also have the same issue on the webform list page "admin/structure/webform" where I have 3 different webforms, but in very particular browser height and depending on the page content, our first webform is inaccessible/hidden on scroll.
Removing the overflow: auto; also seems to solve the issue.

mortona2k’s picture

The patch is adding a scroll event listener. Would it be more performant to use intersection observer?

Eduardo Morales Alberti made their first commit to this issue’s fork.

saschaeggi’s picture

Status: Needs work » Needs review

I've created a new branch with a complete new approach and for me it seems to work locally, but I'd love to gather some more feedback.

Can somebody please test the branch https://git.drupalcode.org/project/gin/-/merge_requests/404?

You can generate a patch to use against the latest dev with https://git.drupalcode.org/project/gin/-/merge_requests/404.diff

Thank you in advance!

anybody’s picture

Thanks @saschaeggi just a note for you and the others: There's a Tugboat link for the MR that can be used for manual testing this easily: "View live preview!"

Great to see progress here, thank you so much! :)

Update: Looks fine in tugboat!

arantxio’s picture

Status: Needs review » Needs work

I've tested this on one of our sites, but I noticed that with a lot of roles, it creates loads of blank space next to the permissions wrapper div.
Adding the css: "table-layout: fixed;" kinda fixes this problem, however now everything gets cramped together which is also not wanted.

saschaeggi’s picture

@Arantxio

it creates loads of blank space next to the permissions wrapper div

Can you share a screenshot?

kinda fixes this problem, however now everything gets cramped together which is also not wanted.

For this as well. Thank you.

saschaeggi’s picture

Status: Needs work » Needs review

I've setup some artificial roles in the linked Tugboat of the MR and can't reproduce the whitespace, see https://mr404-tvh4mpqiaqdfkkufdvfcc1xmmqdtjaxd.tugboatqa.com/admin/peopl... (admin/admin)

arantxio’s picture

StatusFileSize
new168.6 KB
new167.51 KB

Here are some screenshots:

The cramped layout:

The whitespace:

arantxio’s picture

Status: Needs review » Needs work
StatusFileSize
new276.24 KB

I've even tried it on tugboat and here i can also see the whitespace.

saschaeggi’s picture

Status: Needs work » Needs review

@Arantxio I was able to reproduce it in Chrome. I've pushed a fix.

saschaeggi’s picture

Pushed another change to switch to use scrollsync to avoid some of the jerkyness when positioning an element with JS within overflow.

Testers wanted 👀

anybody’s picture

Tugboat is sadly broken (Failed) but the fix works great for me so far, thank you Sascha!

saschaeggi’s picture

hebl’s picture

StatusFileSize
new3.18 MB

Hey @saschaeggi,

The table scrolls horizontally as expected both when in sticky mode and not in sticky mode. So that's all good.

However, I'm experiencing a horizontal scroll outside of the table too. I'll upload a screen recording showing what I mean. [gin-roles-horizontal-scroll.mp4]

Macbook 15", Chrome.

hebl’s picture

Not sure whether this is the best fix, but setting the position attribute of the table to be "relative", before it's then set to "sticky" appears to remove the unwanted horizontal scroll for me in Chrome. See screenshot attached.

saschaeggi’s picture

@Hebl I pushed a fix for Chrome 👀

hebl’s picture

Status: Needs review » Reviewed & tested by the community

Thanks @saschaeggi, I've tested the update and fix is applied. :)

arantxio’s picture

I can confirm that it also works properly on our site now.

robcarr’s picture

StatusFileSize
new53.67 KB

I've applied MR404 mainly to address sticky headers on the permissions page (lots of related - closed - issues), but I actually see 2 sets of table headers with the patch applied. The upper header obscures the 'filter' box on permissions on page load, but is the header that remains sticky when I scroll down.
Screen grab of permissions page

saschaeggi’s picture

@robcarr I can't really reproduce this, can you provide steps how to reproduce this, Browser & which Drupal version you're using. Also does it happen only on a single instance (so potentially another module is interfering?)

bram.velthoven’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new142.42 KB
new111.64 KB

Hi i've tested the patch. And i've found that there are still some things that can use adjustments. On viewport 959px and smaller it prints the sticky header twice, so it seems. Furthermore there seems to be an issue regarding the z-index with the Dropbuttons (operations) having a larger z-index than the sticky header causing overlap.

robcarr’s picture

StatusFileSize
new44.35 KB

@saschaeggi - it's a complex D10.2 site but the issue seems to be down to the margin-bottom value calculated in js > tableheader.js updateTableHeader function by the line:

tableHeader.style.marginBottom = `-${el.querySelector('thead').getBoundingClientRect().height + 1}px`;

In my instance the THEAD has a height of 54px, so the marginBottom value should be 55px, but the JS implements this as in inline style value -54px in the enclosing table. If this value is changed to 20px then the 2 table headers are visible, but the upper, sticky header is above the system 'Filter' input box and it's at least usable.

Screenshot of permissions page with CSS hack

For the moment I'm just going to use some hacky CSS in sites/default/files/gin-custom.css to get around this. Will probably break something else...

.user-admin-permissions .gin--sticky-table-header{
  margin-bottom: 55px !important;
}

(Sorry about the !important value. I know)

saschaeggi credited Murz.

saschaeggi’s picture

saschaeggi’s picture

Status: Needs work » Postponed (maintainer needs more info)

@robcarr how can I reproduce your scenario?

As using your overrides will certainly break it for the general implementation.

ambot112’s picture

StatusFileSize
new732 bytes

The #47 works for minimal item of roles.

saschaeggi’s picture

Status: Postponed (maintainer needs more info) » Needs review

@brian.flores@codeandtheory.com #47 is not a solution we can move forward with. Please test the branch instead 👀

jackfoust’s picture

MR404 has resolved a lot of the issues I was having on various Webform tables.

saschaeggi’s picture

Status: Needs review » Reviewed & tested by the community

Let's get this in! 🎉

saschaeggi’s picture

Status: Reviewed & tested by the community » Fixed

Thank you all so much for collaborating on this one! Good things take time! 😅👏🧡

anybody’s picture

Thank you very very much @saschaeggi, great work!
Looking forward to the next release :)

david.qdoscc’s picture

Sorry, but testing this on the latest 3.x-dev@dev, I see two header rows on the permissions table (one of which is sticky) and no means of scrolling horizontally to roles that overflow the right of the container. Realised that I can actually use Shift+mouse wheel to scroll horizontally on the table!

selwynpolit’s picture

I'm seeing the same. Two header rows but at least the sticky works!

saschaeggi’s picture

Can you share some more information like for example which Drupal version you're using and can you share a screenshot please?

david.qdoscc’s picture

StatusFileSize
new194.42 KB

I'm using Drupal 10.2.6 and PHP 8.2.12. Gin 3.x-dev@dev.

jackfoust’s picture

Latest Gin, Drupal 10.2.6, PHP 8.2.18 - Not able to replicate the double row headers some folks are seeing. Tested with left and top toolbar.

saschaeggi’s picture

@david.qdoscc I'm unable to reproduce this just yet. Did you clear the cache(s)? I would need steps to reproduce this and if there are any other community modules involved which potentially could interfere here.

david.qdoscc’s picture

I've found it! Module Filter is to blame.

saschaeggi’s picture

@david.qdoscc can you open an issue so we can track it there? TY!

david.qdoscc’s picture

saschaeggi’s picture

@david.qdoscc @selwynpolit

I moved the issue to the Module Filter project as the root cause is within that module -> https://www.drupal.org/project/module_filter/issues/3452725

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.