query metadata for reviewcards

[TobiasKampmann]

Preview: https://940-query-metadata-for-re.loculus.org

resolves #940

Summary

Currently exclude status processing and received as the endpoint get-data-to-edit exclude them.

This is a draft to discuss.

Allowing lazy loading for the individual card shows especially when throttling the bandwidth via dev tools. The list appears very fast and will be filled as data arrives. There is currently no indicator of loading yet => draft

By enabling compression server side and with our dummy data, even with sending all data, it works even this way.

Missing: When a sequence entry has a warning the icon should be yellow. Lazy loading makes that not-so-trivial as the information that there is a warning is lazy loaded.

Screenshot

PR Checklist

• All necessary documentation has been adapted.
• The implemented feature is covered by an appropriate test.

[TobiasKampmann]

To circumvent the issues revealed by this ticket, this is a follow-up:
#1000

Most of the changes here will stay, so I suggest to get this merged before working on the next ticket.

[theosanderson]

Congrats on this. I haven't really looked at the code because my brain is fried (off sick).

But I did have a play with it. It looks really good. Do I understand correctly that in the next PR you won't be lazy loading? I think that probably makes sense.

I was initially looking on mobile (which I accept won't be a common use case, but I guess the rest of the site does work on mobile). There, there is some overlapping going on at the right hand side.

I do think we need to use react-tooltip or similar - the native tooltips take too long to appear (here we are using the tooltip to convey real data on the error message, not just a hint of what UI does). Also one can't make the tooltips appear on mobile.

[theosanderson]

Oh and my suggestion would be to put the padlock at the bottom and the tick/cross etc. at the top

[TobiasKampmann]

Thats disappointing with the tooltips, right now we use html capabilities. Less js is always better ;) And it is straight-forward to set just a title...

I guess you didnt discovered the tooltip on the padlock? ;) Should we put the restrictedUntil as a real field if dataUseTerms is restricted?

The overlap stems from the display: absolute. I change that.

[theosanderson]

Hehe, the best experience for our users is always better (and I agree that for many purposes that will be HTML tooltips, but in this case we're not using the tooltip conventionally, we're using it to display a message that changes( e.g. the "Swizzerland is not a country"). So users, however well they know our website, have to trigger the tooltips, which isn't the use case that HTML tooltips are designed for

[theosanderson]

Your colours on the edit page - text-red-600 - are really nice - we should use them here too :)

[theosanderson]

Yes, displaying restrictedUntil sounds good!

[corneliusroemer]

I agree with @theosanderson that the standard tooltip delay is too long

There's a layout issue when submitter ids are of differing lengths:

Also layout shift here - I guess we need some sort of (hidden) table to keep things aligned more broadly across sequences

[TobiasKampmann]

But we display the full error message, too.. So execpt for the padlock-restrictedUntil info there is no essential info behind a tooltip right now...

[TobiasKampmann]

The delay is not customizable. I forfeit and reintroduce a js lib for that ;)

[corneliusroemer]

We might want to show tooltips for all sorts of things so we need a library eventually anyways. I agree that in this case here they might not be essential (yet) but that's just a question of time.

[theosanderson]

I don't think we have to keep things aligned across sequences. If there is a super long location in one field then all the other sequence tiles will be mostly empty space. For me the core of this looks good. We can definitely see if we can make improvments downstream.

[TobiasKampmann]

the overlapping issue is a bit harder...

I would suggest to make a "linebreak" after the accession/submission Id block. This way the buttons as they are should be overlap-free. Maybe one have to limit the width of the id-block, too.

[theosanderson]

Let's not add a line break if you mean that will be visible on the page. We can definitely make this work with flexbox and stuff. Feel free to leave it as is with the overlap and raise an issue for me to fix the overlap

[TobiasKampmann]

I found a solution. We use a bit more space on small displays, but it looks very good ... in my opinion :D

[TobiasKampmann]

Its not perfect though... There is a width, where the break doesnt appear, but stuff overlap. But as a first draft, I suggest, this is fine for now.

[theosanderson]

Sounds good. I'll let Fabian comment on the code as my brain is still not really working and in any case I struggle with Kotlin, but appearance looks good to merge and we can tweak later.

[fengelniederhammer]

Looks good!

[theosanderson]

(BTW, after submitting 500 sequences if I move to another tab and then back to the review page the page is fully white for like 5 seconds or more. But I presume that will change with pagination etc.)

[corneliusroemer]

Unusable when there are ~1000 sequences, but this will get better with pagination

I thought this would be ok, but now I'm stuck with 1000 sequences and browser freezes totally making preview unusable.

For now, could we somehow limit the number of cards shown to prevent browsers from freezing when someone uploads 1000 sequences (which does happen in testing)?

I managed to dig myself out of the hole by using: https://940-query-metadata-for-re.loculus.org/dummy-organism/user/seq (the old submit view) which didn't crash. So now we're back to usable - but would be best to avoid having something that freezes with modest numbers of sequences on main.

[theosanderson]

Unusable when there are ~1000 sequences, but this will get better with pagination

But Tobias has acknowledged that (in general terms) and suggested we defer that fix until #1000.. #994 (comment) that seems fine. That should entirely change things since the website should only ever have knowledge of 50-100 sequences at a time, and otherwise just a number of pages

IMO we should go for whatever makes development the fastest and doesn't lead to a worse final outcome

[corneliusroemer]

Approving as there's a workaround via the old review page (without that one would be stuck indefinitely with a crashing page)

[TobiasKampmann]

@theosanderson currently the restricted until is in the tooltip of the padlock.

Should I just add restrictedUntil as a metadataField, i.e., displaying it as such.

[theosanderson]

@theosanderson currently the restricted until is in the tooltip of the padlock.

Should I just add restrictedUntil as a metadataField, i.e., displaying it as such.

IMO your current approach is good!

[TobiasKampmann]

Then there is only one thing left:
When data is loaded and a seq is AWAITING_APPROVAL will switch its check icon from green to yellow when warnings are loaded.

[fengelniederhammer]

Looks good