@katebutler opened this Pull Request on June 30th 2019 Member

Fixes #14333

@katebutler commented on June 30th 2019 Member

There are at least two issues:

  • Sometimes the second column disappears behind the first (frozen one)
  • The content in the <th> doesn't scroll with the table body
@katebutler commented on July 1st 2019 Member

The first column in the tbody is frozen and won't move when user scrolls. The first column in the thead will move, as freezing this causes several layout issues (second column disappearing under first, jagged edges on bottom of thead when some ths are different heights, long labels in first column of data overlying the values in other columns).

@katebutler commented on July 2nd 2019 Member

We need the first column in the thead to be frozen as well. This causes the second column to display over the top of the first because of the absolute positioning.
One solution to this is to set the padding-left on the second column to the same as the width of the first column. I'm working on the JS to keep these in sync when the window is resized etc.

@katebutler commented on July 5th 2019 Member

Now using position: sticky which solves most of the issues with absolute positioning. This is not supported on table elements in IE or Safari but I've tested these on BrowserStack and they degrade nicely to the existing behaviour so I think that's OK.
One remaining issue is that the border-bottom between each row is missing on the first column in Firefox and Edge.

@tsteur commented on July 5th 2019 Member

For safari you can use position: -webkit-sticky (worked for me on safari)

@katebutler commented on July 7th 2019 Member

The labels are very wide when the datatable is quite wide but doesn't have many columns (e.g. on the Goals Overview page, or the Search Categories report on the Site Search page). This is due to the datatable itself taking up 100% of the available width, then calculating how much the data columns need and assigning all of the excess to the label column. Not sure what the best approach here is, should we try to spread the width more evenly over the columns or should we reduce the width of the datatable?

@katebutler commented on July 8th 2019 Member

Have tested successfully in Chrome/Firefox/Edge on Windows, Safari on MacOS, Safari on iOS, Chrome/Firefox on Android. Fails gracefully with IE or Safari on Windows, reverting to current behaviour where the whole datatable scrolls.

@tsteur commented on August 1st 2019 Member

@katebutler looking great! Noticed on issue which you can also see here: https://github.com/matomo-org/matomo/pull/14594/files?short_path=c74ac3c#diff-570cf8d0cd615f4d8f62314cf92bed8a

There are sometimes some row action icons missing, eg the row evolution.

When I highlight the element, I noticed there's not enough space so it shows some icons in a "new line" I think and therefore isn't visible. Before it would have probably simply "overflown" it and shown it in the next column.

When I lower here the margin-left and set it to say 284px, then all the row icons become visible:
It seems the calculation for that margin-left needs to be adjusted slightly to move the icons further to the left. Yes, there will be a lot of updating screenshots again unfortunately :(

@tsteur commented on August 6th 2019 Member

@katebutler there are a few more failing ui tests before we can merge: https://builds-artifacts.matomo.org/matomo-org/matomo/3.x-dev/35329/

Maybe we only need to update the expected screenshots?

@tsteur commented on August 6th 2019 Member

Apart from this looks good 👍

@tsteur commented on August 13th 2019 Member

Awesome @katebutler 💯

This Pull Request was closed on August 13th 2019
Powered by GitHub Issue Mirror