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

Add position:sticky to Experiment table head #1805

Merged
merged 4 commits into from
Jun 2, 2022
Merged

Conversation

rogermparent
Copy link
Contributor

@rogermparent rogermparent commented May 31, 2022

This PR adds a simple implementation of a sticky header using CSS position: sticky to implement it.

sticky-header-demo.mp4

The style is currently imperfect, particularly on the right-hand side when the table doesn't expand to the full width horizontally. Ther e is also currently no shadow added when scrolling.

I first tried to use a Flexbox-based solution thinking it more proper as we've had some trouble with position:sticky on websites, but the flex implementation took a lot of wrestling with the table widths and ended up being less user-friendly as horizontal scrolling became more difficult in a vertical-scroll-only body.

Relates to #1562

@rogermparent rogermparent self-assigned this May 31, 2022
@rogermparent rogermparent added A: experiments Area: experiments table webview and everything related 🎨 design Needs design input or is being actively worked on product PR that affects product labels May 31, 2022
@rogermparent rogermparent changed the title Add position:sticky to thead Add position:sticky to Experiment table head May 31, 2022
position: sticky;
top: 0;
z-index: 3;
background-color: $bg-color;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This keeps the right border of the table body from overlapping with the head

Copy link
Member

@mattseddon mattseddon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

We should also make the first column (experiments) sticky as the user cannot drag + drop. IMO timestamp is less important.

Can you follow up, please and thank you.

@mattseddon mattseddon requested review from maxagin and shcheklein June 2, 2022 01:00
@mattseddon mattseddon enabled auto-merge (squash) June 2, 2022 01:00
@shcheklein
Copy link
Member

Thanks @rogermparent for addressing this. Agreed with @mattseddon and one more next step is to add a bit of shadow if possible / if it makes sense (when some content is hidden). But definitely let's merge this!

@codeclimate
Copy link

codeclimate bot commented Jun 2, 2022

Code Climate has analyzed commit 6138da2 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (85% is the threshold).

This pull request will bring the total coverage in the repository to 96.8% (0.0% change).

View more on Code Climate.

@mattseddon mattseddon merged commit d6ea272 into main Jun 2, 2022
@mattseddon mattseddon deleted the sticky-table-header branch June 2, 2022 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: experiments Area: experiments table webview and everything related 🎨 design Needs design input or is being actively worked on product PR that affects product
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants