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

Make the scoreboard summary sticky #2132

Merged
merged 1 commit into from
Mar 24, 2024

Conversation

vmcj
Copy link
Member

@vmcj vmcj commented Aug 30, 2023

Before I fully fix the PR I would like to know if we want this.
The reason I like it is that the summary per sortorder is now properly used and when scrolling the scoreboard you get all the relevant information directly.

Current problems still:

  • Rewrite the commit messages to make sense
  • Move everything out of inline style into dynamic internal <style>
  • due to the display: relative the background jumps over the border (probably together with the border collapse) so we should use shadows as border & shadow misalign on 1px (a shadow can not fall with at 12 o'clock )

@meisterT
Copy link
Member

meisterT commented Sep 3, 2023

In general I like it on my large screen. If I shrink it to mobile view with Chrome developer tools it is appearing some time in the middle of scrolling down the scoreboard though - not sure if this is also happening on real devices.

Also I wonder whether we can somehow visually indicate in a nice way that there is more stuff behind the sticky part?

@vmcj
Copy link
Member Author

vmcj commented Sep 3, 2023

In general I like it on my large screen. If I shrink it to mobile view with Chrome developer tools it is appearing some time in the middle of scrolling down the scoreboard though - not sure if this is also happening on real devices.

That could be desired behaviour, we have a summary per sortorder so it should stick keep with its part of the table. Another sticky part should than come up for the next sortorder though.

Also I wonder whether we can somehow visually indicate in a nice way that there is more stuff behind the sticky part?

I think technically I can with CSS indicate that, the only question is how.

@meisterT
Copy link
Member

meisterT commented Sep 3, 2023

I only had one sortorder and had to scroll down ~1 screen to make the sticky part appear.

@vmcj vmcj marked this pull request as draft September 3, 2023 10:48
@vmcj
Copy link
Member Author

vmcj commented Sep 3, 2023

I only had one sortorder and had to scroll down ~1 screen to make the sticky part appear.

Than I'll investigate, the PR was only as a quick demo if we would want this.

@vmcj
Copy link
Member Author

vmcj commented Feb 16, 2024

Also I wonder whether we can somehow visually indicate in a nice way that there is more stuff behind the sticky part?

I think technically I can with CSS indicate that, the only question is how.

https://codepen.io/TomAnthony/pen/qBqgErK is the best thing, https://stackoverflow.com/a/25312347 explains why this is very hard to achieve.

@vmcj
Copy link
Member Author

vmcj commented Feb 16, 2024

I only had one sortorder and had to scroll down ~1 screen to make the sticky part appear.

Checking on my android the header row is also not sticky, I assume you're on android also? The easiest fix would be to not apply this specific styling on phones.

@meisterT
Copy link
Member

I didn't try it on an actual phone but with the device toolbar of the chrome dev extensions.

@vmcj vmcj force-pushed the footer_summary_scoreboard branch 2 times, most recently from cca5940 to 469525b Compare February 17, 2024 15:58
@vmcj
Copy link
Member Author

vmcj commented Feb 17, 2024

Verified on phone (FF+Android) and it seems to work, works on an older iPhone.

If approved I'll squash this.

@vmcj vmcj marked this pull request as ready for review February 17, 2024 16:13
@meisterT
Copy link
Member

In Chrome / Desktop it does work. In Chrome / Desktop with developer tools set to mobile devices it doesn't work (regardless of the device). In Firefox / Desktop it doesn't work either for me. In Firefox / Desktop with developer tools it also doesn't work.

@vmcj vmcj force-pushed the footer_summary_scoreboard branch from bf8c13e to 8b71fc8 Compare February 24, 2024 12:45
@vmcj
Copy link
Member Author

vmcj commented Feb 24, 2024

In Chrome / Desktop it does work. In Chrome / Desktop with developer tools set to mobile devices it doesn't work (regardless of the device). In Firefox / Desktop it doesn't work either for me. In Firefox / Desktop with developer tools it also doesn't work.

I can't reproduce this.

My PR doesn't add anything for phones/small screens as there the sticky header also doesn't work (and is left for a future PR). For me it works in chromium & firefox (both on desktop).

@vmcj vmcj force-pushed the footer_summary_scoreboard branch from d63f1ad to 87e591b Compare March 23, 2024 10:41
This bundles some of the inlince CSS and adds a z-index per sortorder to
make the sliding natural.

In case of the same z-index the order of creation in the DOM is picked
as how to stack. Given that creating the DOM from bottom to top is
unmaintable the less worse option is introducing a lot of z-indexes.

Perfect solution would be to put this in a class to remove the inline
styles but that is left for another time.

Unrelated to this PR, it seems in chromium based browser the
border/shadow of the sticky elements gets lost.

The misalignment of the `tbody` is because we check for the sortorder,
when there are multiple we close and open a new one which looks
misaligned due to the twig `if` condition.
@vmcj vmcj force-pushed the footer_summary_scoreboard branch from 52f0f1b to 9af668c Compare March 24, 2024 15:49
@vmcj vmcj added this pull request to the merge queue Mar 24, 2024
Merged via the queue into DOMjudge:main with commit 1da1ff5 Mar 24, 2024
19 of 20 checks passed
@vmcj vmcj deleted the footer_summary_scoreboard branch March 24, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants