-
Notifications
You must be signed in to change notification settings - Fork 112
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
Optional "Three letter" Amino-Acid badges for beginners #323
Comments
Hi @chatelao - Thanks for using CoVariants and thank you for this suggestion! I agree, this would be a very cool addition. Unfortunately at the moment we don't have the resources to implement this, as our project is pretty small. However, if someone else would like to work on this, I'd welcome PRs to add this feature! |
Hi, I'm interested in this issue and project, I'll take a look at it and let you know of my advances |
Thanks @carlos-montano-hub - that would be great! It would be fantastic to have some kind of toggle in the header or footer of pages that could switch this on/off across the site :) |
Whenever it helps @carlos-montano-hub - Here the full list as "csv"
|
If the PR will pass, a simple lookup table should be available @carlos-montano-hub in the codebase: |
In terms of implementation, one could:
A good example of an existing toogleable recoil atom is the "tooltip sorting" (it's not boolean, but close): covariants/web/src/state/TooltipSort.ts Lines 8 to 14 in 55e9567
And the usage is here covariants/web/src/components/ClusterDistribution/ClusterDistributionPlotTooltip.tsx Line 54 in b253cd9
and here:
It's not really feasible to pass a prop to every single badge component - there are too many. But with recoil, this value can be "global" and be queried or set by any badge component. |
My concern is though that the badges with long AA names may become much bigger and they may not fit stylistically in certain places, like various lists, tables and sidebars. And this might be a much harder problem to solve, than passing a boolean or mapping 1-letter to 3-letter codes. Neither I ever saw 3-letter codes used anywhere when describing mutations. But I'll reserve judgement to Emma here. If I remember correctly, we already have tooltips for these badges. Perhaps all this beauty can just go into tooltips? |
Of course always a little hard to judge in static pictures, but so far this looks pretty good to me? It's a little more crowded but doesn't seem to be messing up the layout extremely, and as it'll be an option, people can toggle if seeing these is a real win compared to a slightly more tidy layout. If you submit a PR when you're ready @carlos-montano-hub , it'll generate a preview that we can explore to see how it looks on a variety of resolutions & devices! |
@ivan-aksamentov Does make a good point that the full amino-acid names are visible on mouseover with the tooltips, but if this would be genuinely useful for people to see more easily, and if we can do it without messing up the layout (which so far looks pretty ok to me) then I think it could be worth putting in. Let's see how a preview looks! |
I love the screenshot of the three letter variant. |
IS
COULD
https://github.com/hodcroftlab/covariants/blob/b253cd94e4dcf4f32b3f7667717304d086c16141/web/src/components/Common/MutationBadge.tsx
The text was updated successfully, but these errors were encountered: