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

feat(website): simple mutation badges #1666

Merged
merged 5 commits into from
Apr 29, 2024
Merged

Conversation

anna-parker
Copy link
Contributor

@anna-parker anna-parker commented Apr 26, 2024

resolves #1620

preview URL: https://feat-simple-mutation-badg.loculus.org/

Summary

This PR makes it easier to view lists of substitutions by creating a mutation badge for each mutation.

  • It stops showing substitutions as a list of strings and now uses the processed substitution information from the backend (added to endpoint in feat(website): Update Lapis client endpoints #1662).
  • It uses the SubBadge component first defined in feat(web): add mutation badges #1446 to turn each mutation into a badge object. This can be extended to multiple segment data.
  • Badges are listed with spaces to enable search and copying of sequence lists.
  • Badge colors have been desaturated to enable easier reading.

Screenshot

image

Future work

  • Handle how large substitution lists are shown
  • Tweak color scheme
  • Decide how best to show genes

PR Checklist

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

@anna-parker
Copy link
Contributor Author

anna-parker commented Apr 26, 2024

The last commit causes issues with the formatting when running the e2e tests on docker:
image.

This is exactly what I saw in the feat/mutation-badges2 PR - this indicates the issue is not linked to any package updates but potentially the direct access and modification of import { type CSSProperties, useMemo } from 'react';.

The odd thing is locally (and on argocd) the pages look fine:
image

@anna-parker anna-parker added the preview Triggers a deployment to argocd label Apr 26, 2024
Copy link
Member

@chaoran-chen chaoran-chen left a comment

Choose a reason for hiding this comment

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

When selecting and copying the list of mutations, I get:

A296CC357TG361AT370CT378CT379CG382AT386C

It would be nice if there would be a space in between.

@corneliusroemer
Copy link
Contributor

Colors are maybe too saturated/dark (sorry for bikeshedding) - making them fairly light would help with a) readability of A/C/T/G and b) make it fit better into loculus' fairly light color scheme.
image

@corneliusroemer
Copy link
Contributor

ChatGPT does this really well: https://chat.openai.com/share/1fdf940b-6410-421b-926f-3d9b2af64b87

Writes a Python script to convert hex to hsv, then adjusts saturation and brightness, then converts back to hex!

Here's the result (you can use the script to tweak manually, it provides the exact array you can just copy/paste:
image

@anna-parker
Copy link
Contributor Author

ChatGPT does this really well: https://chat.openai.com/share/1fdf940b-6410-421b-926f-3d9b2af64b87

Writes a Python script to convert hex to hsv, then adjusts saturation and brightness, then converts back to hex!

Here's the result (you can use the script to tweak manually, it provides the exact array you can just copy/paste: image

I have actually changed the colors to be more loculus like - was all manual so we can also not use ?
image

@anna-parker
Copy link
Contributor Author

The last commit causes issues with the formatting when running the e2e tests on docker: image.

This is exactly what I saw in the feat/mutation-badges2 PR - this indicates the issue is not linked to any package updates but potentially the direct access and modification of import { type CSSProperties, useMemo } from 'react';.

The odd thing is locally (and on argocd) the pages look fine: image

I think this might be a timeout issue - I created a branch off this branch and only showed a single nucleotide mutation in badge form - there the tests succeeded, however I see there was a warning that they were slow:
image

@theosanderson
Copy link
Member

(IMO some of those manual colours are oversaturated and of those two options my pref would maybe be Cornelius's)

@anna-parker
Copy link
Contributor Author

Our e2e tests are very slow - the image we see is probably just the html preview:
image
we only start loading the page 24s seconds into the test and the time to render the page is almost 30s. The default for playwright is only 30s per test. We need to extend this (or run on faster machines).

@anna-parker anna-parker force-pushed the feat/simple-mutation-badges branch 4 times, most recently from 7e9162a to 5f892e0 Compare April 26, 2024 20:52
@anna-parker anna-parker marked this pull request as ready for review April 26, 2024 21:23
Copy link
Member

@theosanderson theosanderson left a comment

Choose a reason for hiding this comment

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

Looks nice!

@anna-parker anna-parker force-pushed the feat/simple-mutation-badges branch from 135bf82 to a43c2b9 Compare April 29, 2024 12:28
@anna-parker anna-parker force-pushed the feat/simple-mutation-badges branch from a43c2b9 to b88fdb5 Compare April 29, 2024 13:11
@anna-parker anna-parker force-pushed the feat/simple-mutation-badges branch from d876fce to 8a4bf42 Compare April 29, 2024 14:36
@theosanderson theosanderson changed the title Feat/simple mutation badges feat(website): simple mutation badges Apr 29, 2024
Copy link
Member

@chaoran-chen chaoran-chen left a comment

Choose a reason for hiding this comment

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

Looks great to me - thanks!

@anna-parker anna-parker merged commit 7324bc1 into main Apr 29, 2024
12 checks passed
@anna-parker anna-parker deleted the feat/simple-mutation-badges branch April 29, 2024 16:07
@anna-parker anna-parker mentioned this pull request May 7, 2024
2 tasks
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
5 participants