-
Notifications
You must be signed in to change notification settings - Fork 2
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
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
backend/src/main/kotlin/org/loculus/backend/controller/SubmissionController.kt
Outdated
Show resolved
Hide resolved
backend/src/main/kotlin/org/loculus/backend/service/submission/SubmissionDatabaseService.kt
Show resolved
Hide resolved
There was a problem hiding this 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
2fa1253
to
1e7f6e9
Compare
There was a problem hiding this 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

Turned into issue because not blocking: #1447
1e7f6e9
to
b1639c2
Compare
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. |
This isn't a blocking issue
There was a problem hiding this 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
Will indeed merge, as I've taken Fabian's advice |
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 thegetSequences
call, which is applied initially, and applies to the counts. Then the second main filter is applied after the counts.Remaining issue: #1372