Skip to content
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

Freeze left-hand column of data table #14594

Merged
merged 26 commits into from Aug 13, 2019
Merged

Freeze left-hand column of data table #14594

merged 26 commits into from Aug 13, 2019

Conversation

katebutler
Copy link

Fixes #14333

@katebutler katebutler added the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label Jun 30, 2019
@katebutler katebutler added this to the 3.11.0 milestone Jun 30, 2019
@katebutler
Copy link
Author

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
Copy link
Author

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 katebutler added Needs Review PRs that need a code review and removed Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. labels Jul 1, 2019
@katebutler katebutler added Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. and removed Needs Review PRs that need a code review labels Jul 2, 2019
@katebutler
Copy link
Author

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
Copy link
Author

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
Copy link
Member

tsteur commented Jul 5, 2019

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

@katebutler
Copy link
Author

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
Copy link
Author

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.

@katebutler katebutler added Needs Review PRs that need a code review and removed Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. labels Jul 14, 2019
@mattab mattab modified the milestones: 3.11.0, 3.12.0 Jul 23, 2019
@tsteur
Copy link
Member

tsteur commented Aug 1, 2019

@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:
image
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 :(

katebutler added 4 commits August 2, 2019 13:25
# Conflicts:
#	plugins/Dashboard/tests/UI/expected-screenshots/DashboardManager_removed.png
#	plugins/Dashboard/tests/UI/expected-screenshots/Dashboard_loaded_token_auth.png
#	plugins/Dashboard/tests/UI/expected-screenshots/Dashboard_removed.png
#	tests/UI/expected-screenshots/PivotByDimension_pivoted_columns_report.png
#	tests/UI/expected-screenshots/Theme_home.png
#	tests/UI/expected-screenshots/UIIntegrationTest_dashboard1.png
#	tests/UI/expected-screenshots/UIIntegrationTest_ecommerce_sales.png
@tsteur
Copy link
Member

tsteur commented Aug 6, 2019

@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
Copy link
Member

tsteur commented Aug 6, 2019

Apart from this looks good 👍

@tsteur tsteur merged commit 8593219 into 3.x-dev Aug 13, 2019
@tsteur tsteur deleted the 14333 branch August 13, 2019 23:43
@tsteur
Copy link
Member

tsteur commented Aug 13, 2019

Awesome @katebutler 💯

@sgiehl sgiehl mentioned this pull request Nov 9, 2020
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Data table labels should stick to the left
3 participants