Navigator View Implementation + Additions to tutorial.md + generalized view/gene switching functionality in Views folder #156
Conversation
…into NavigatorView
…and nodes are aligned, tree drawn nicer, data is correct to the API.
…eys to nodes. Watering can now shown.
…nk is clicked. Changes to placeholder files as well. Changes to package.json and eslint file to include eslint tsdoc checking now
Eplant/tutorial.md
Outdated
There was a problem hiding this comment.
Expecting this to change due to routing change
There was a problem hiding this comment.
Yes, this will be different and will have to be reviewed again
| @@ -0,0 +1,16 @@ | |||
| import { styled } from '@mui/material' | |||
| export default styled(function CellEFPIcon(props) { | |||
There was a problem hiding this comment.
Rename this file and export to NavigatorViewIcon
There was a problem hiding this comment.
Not entirely sure what you mean by export to NavigatorViewIcon, would you be able to elaborate? Otherwise, I have changed line 2 to match the other styles of just styled((props) => { as well as the file name and import name in other files.
| import { useViewSwitch } from '../ViewGeneSwitching' | ||
|
|
||
| import CellEFPIcon from './Icons/CellEFPIcon' | ||
| import GeneInfoViewIcon from './Icons/GeneInfoViewerIcon' /** Placeholder icon for those that are not yet implemented in ePlant3 */ |
There was a problem hiding this comment.
Rename this as there is another "GeneInfoViewIcon"
There was a problem hiding this comment.
Rename this file as there is another GeneInfoViewIcon.
There was a problem hiding this comment.
Rename this to avoid naming conflict with other cellefpicon
There was a problem hiding this comment.
Did you use this coding pattern from Yonah? Doesn't seem like other icons use it. We seem to be using the GeneInfoViewerIcon.tsx style. Clever use of the 'currentColor' though. I think I can accept this but we need to talk about this during a standup or you can correct it.
There was a problem hiding this comment.
I did follow his structure yeah. I will try and change this in the meantime before standup
There was a problem hiding this comment.
This is changed now
There was a problem hiding this comment.
Same renaming conflict issue, make it clear it is for navigator. Or even better can we use the already built efp icon?
There was a problem hiding this comment.
The reason I am using my own, is due to some custom sizing I needed to do without messing with the original
There was a problem hiding this comment.
Let me know if there is a way to achieve the same goal without using a separate file. For example, in some cases I needed to change width and height of the icon.
Eplant/views/ViewGeneSwitching.tsx
Outdated
There was a problem hiding this comment.
Keeping this for now, assuming it will change a lot after routing merge.
There was a problem hiding this comment.
Correct, it is gone after the routing changes.
package.json
Outdated
There was a problem hiding this comment.
What are these packages for ? They're not explicitly imported:
- msw
- react-query
- react-helmet
- phylotree (I think we don't use this anymore?)
- react-d3-tree (is this used by d3?)
There was a problem hiding this comment.
Good catch, these are no longer needed. They were previously needed but I forgot to remove them manually.
There was a problem hiding this comment.
msw was in staging, so my change is just using an updated version. Should I keep that change? I've removed the others as they are no longer needed.
| 'https://ensembl.gramene.org/Medicago_truncatula/Gene/Summary?g={geneName}', | ||
| TOMATO: | ||
| 'https://ensembl.gramene.org/Solanum_lycopersicum/Search/Results?species=Solanum_lycopersicum;idx=;q={geneName}', | ||
| POTATO: '', // Placeholder for N/A |
There was a problem hiding this comment.
I did not find a dedicated link for potato last time I checked.
There was a problem hiding this comment.
Assuming some of the URL fetching here will change with routing ? I.e. make sure this checks if the gene is defined in the URL or maybe yukthiw's change will already define it as genetic element. either way check if this breaks after routing PR.
There was a problem hiding this comment.
Not to worry, the fetching is gone in my routing changes file.
| * @param metadata - Additional tree metadata including expression data | ||
| * @param primaryGene - Identifier of the primary gene being analyzed | ||
| * @param species - Species name for the primary gene | ||
| * @returns A D3-compatible tree structure |
There was a problem hiding this comment.
Give us a sample D3 tree for the return, make it small
There was a problem hiding this comment.
Adding this to the description:
Example:
newickToD3("(A:0.1,B:0.2):0.3;", { efp_links: { A: "linkA", B: "linkB" }, genomes: { A: "SOYBEAN", B: "RICE" }, SCC_values: { A: 0.9, B: -0.2 }, sequence_similarity: { A: 95, B: 88 }, maximum_values: { A: 1.0, B: 1.0 }, tree: "(A:0.1,B:0.2):0.3;" }, "A", "SOYBEAN")
Returns: { name: 'internal', value: 0.3, children: [{ name: 'A', value: 0.1, metadata: { genome: 'SOYBEAN', efp_link: 'linkA', scc_value: 0.9, sequence_similarity: 95 } }, { name: 'B', value: 0.2, metadata: { genome: 'RICE', efp_link: 'linkB', scc_value: -0.2, sequence_similarity: 88 } } ] }
There was a problem hiding this comment.
It will look nicer in the comment...
| } | ||
| } | ||
|
|
||
| /** Calculate dimensions based on number of leaf nodes */ |
| * @param props - Component properties | ||
| * @returns JSX element containing metadata visualizations | ||
| */ | ||
| const MetadataVisualizations = ({ |
There was a problem hiding this comment.
This is important fn, describe the params inside the props obj
Full changes for the navigator view(any improvement suggestions welcomed). Also some additions to the tutorial document to include things like code commenting, view switching, etc. Added a module in the Views folder to be used by any view to switch view, gene, or both. Instructions on use are in the tutorial.
NOTE: These changes are based on pre-routing PR modifications. This should only be merged if the routing PR is still un-merged. A separate PR for post-routing changes will be available later that also changes how the view is implemented + tutorial.