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): Split mutations by gene (for aa) and segment (for nucs), fix "show more" bug #1811

Merged
merged 2 commits into from
May 8, 2024

Conversation

anna-parker
Copy link
Contributor

@anna-parker anna-parker commented May 8, 2024

resolves #1796, resolves #1620

preview URL: https://feat-split-mutations-by-s.loculus.org/

Summary

  • This PR splits mutations up by segments (currently the only segments we have are genes but in future this can be used for segmented genomes) to allow for easier viewing.
  • It also removes the show more button for segments that have under MAX_INITIAL_NUMBER_BADGES and where the button is not required.
  • It visually removes the segmentName from the start of the mutation badge object (as this is now clear from the title). However, the sequenceName is still in the mutationBadge component and when the mutations are copied the sequenceNames are still appended to the front of the mutation: i.e. GP:A83V GP:T263A... additionally, the page can be searched with appended sequenceName.

Screenshot

image

@anna-parker anna-parker added website Tasks related to the web application preview Triggers a deployment to argocd labels May 8, 2024
In order to do this I create a segmentedMutations object which maps a segment to a list of all mutations on that segment.
@anna-parker anna-parker force-pushed the feat/split_mutations_by_segment branch from f1a225f to 34f2937 Compare May 8, 2024 13:04
@anna-parker anna-parker marked this pull request as ready for review May 8, 2024 13:10
@corneliusroemer corneliusroemer changed the title feat(website): Split mutations by segment feat(website): Split amino acid mutations by gene, fix show more bug May 8, 2024
@corneliusroemer corneliusroemer changed the title feat(website): Split amino acid mutations by gene, fix show more bug feat(website): Split mutations by gene (for aa) and segment (for nucs), fix "show more" bug May 8, 2024
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 great!

I was wondering if you want to not include the gene/segment in the badges to make the more concise now that genes/segments are included in the title.

So not show VP35: in the badges VP35:A13V. Ideally this would still be what the user copies if they use the clipboard but that's not super important if it's tricky.

Brave Browser 2024-05-08 16 32 18

Questions:

  • What happens for segments/genes without mutations? Are they omitted or do we see lots of headings with no info?
    Ok here's the answer: they are just not shown 👍
Brave Browser 2024-05-08 16 34 57

@anna-parker
Copy link
Contributor Author

anna-parker commented May 8, 2024

@corneliusroemer - great idea - think this should be possible with CSS Off-Screen Positioning - I will just need to figure out how to do this with tailwind - I will try to see if I can do this quickly or I will create a follow up issue.

Update: tailwind has the screenreader-only class which is intended just for this use case.

… - this allows the mutations to be searchable with an appended gene name (and will be visible when the amino acid mutations are copied) but it will not be shown on the page.
@anna-parker anna-parker force-pushed the feat/split_mutations_by_segment branch from 357bc72 to 6d45fe3 Compare May 8, 2024 15:41
@anna-parker anna-parker merged commit 9106d31 into main May 8, 2024
13 checks passed
@anna-parker anna-parker deleted the feat/split_mutations_by_segment branch May 8, 2024 16:54
@anna-parker anna-parker restored the feat/split_mutations_by_segment branch May 8, 2024 16:59
@corneliusroemer
Copy link
Contributor

Quick Q:
I don't see the gene names ommitted on main now that this is merged
image

Did something go wrong with this?
image

@anna-parker

Also, maybe the substitutinos header should be bolder than the individual segments, right now GP looks like higher heading than "substitutions"

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 website Tasks related to the web application
Projects
None yet
3 participants