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

Review page: Display counts of different sequences statuses and allow filtering and improve loading CLS and restyle buttons #1371

Merged
merged 9 commits into from
Mar 22, 2024

Conversation

theosanderson
Copy link
Member

@theosanderson theosanderson commented Mar 17, 2024

resolves #1161, resolves #1034

preview URL: https://reviewcounts.loculus.org

Get the review page to display counts of sequences in different statuses and to allow toggling their visibility. Cache the old result and used it when grayed out when loading to avoid jumping to a totally white loading screen.

To get this to work we had to add an initialStatusFilter to the getSequences call, which is applied initially, and applies to the counts. Then the second main filter is applied after the counts.

image

Remaining issue: #1372

@theosanderson theosanderson added the preview Triggers a deployment to argocd label Mar 17, 2024
@theosanderson theosanderson requested review from corneliusroemer and chaoran-chen and removed request for chaoran-chen and corneliusroemer March 17, 2024 20:46
@theosanderson

This comment was marked as outdated.

Copy link
Contributor

@corneliusroemer corneliusroemer left a comment

Choose a reason for hiding this comment

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

Great, I like the design and functionality - I haven't looked at the code

Copy link
Contributor

@corneliusroemer corneliusroemer left a comment

Choose a reason for hiding this comment

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

Tooltip z score is not right, or the other way round, the top bar is too far in front

image

Turned into issue because not blocking: #1447

@theosanderson
Copy link
Member Author

It isn't obvious to me that the tooltip z-index is "wrong" per se. It does look ugly here because the fixed header doesn't have 100% width. Otherwise I think it would be fine. Generally the design would be better if the fixed header had 100% width (the shadow that cuts off looks weird). But this is low priority for now. (It also isn't, as I understand it, easy to change the tooltip z-index). So not going to address that for now.

Copy link
Contributor

@corneliusroemer corneliusroemer left a comment

Choose a reason for hiding this comment

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

This is really nice, will leave approval in case you want to merge already before Fabian has time to review - I guess post-merge review always works as well

@theosanderson
Copy link
Member Author

Will indeed merge, as I've taken Fabian's advice

@theosanderson theosanderson merged commit a996ba2 into main Mar 22, 2024
13 checks passed
@theosanderson theosanderson deleted the reviewcounts branch March 22, 2024 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Triggers a deployment to argocd
Projects
None yet
3 participants