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

Chromosome view yonah #131

Merged
merged 40 commits into from
Aug 8, 2024
Merged

Conversation

y330
Copy link
Collaborator

@y330 y330 commented Jul 4, 2024

For yukthi to look at:
Eplant/views/ChromosomeViewer/Viewer/GeneList.Tsx Line 111

y330 added 15 commits June 17, 2024 15:11
…mosomes, using the species state, and provide the chromosomes as a prop to the component
the fetch is not working to get the chromosomes
- hardcoded the chromosome data to the component so i could at least see
the view working, and the getGenes fetch function is working fine
- so there must be something wrong with my getintitialdata function
=================
-[X] styled: genelist popup box, start and end of the range based on click, padding of items on the genelist
-[X] added: gene info popup(visible on clicking gene in genelist popup box), chromosome name and size labels
TODO
-[] add lines starting at the top and bottom popup box that converge onto the specific location of the chromsome when clicked
-[] fix outside click of gene info popup closes the all popups, and when you click inside the geneinfo popup it closes TODO
-[] fix getInitialData so that it fetches the chromosomes, currently just using hardcoded chromosome object
-[] tweak more styling
…s much better, made info popup draggable, possible to interact with without closing and more
- fixed GeneInfoPopup so thats its draggable and can be opened and closed properly
- TODO
 + implement fetchGeneSummary function in Chromosome.jsx to get active gene location from geneticElement prop
 + implemnet setActiveGene in GeneList and GeneInfoPopup to allow the user to load genes into the collection on the sidebar
…x.tsx instead of using a third party library

- added view actions
- touched up the view functions and made it more functional.
- added functionality for changing the gap between chromosomes depending on how many there are. i.e. if its Populus_trichocarpa
- added gene aliases to genelist popup
- switched back to MapInteractionCSS component for zooming because panzoom was causing bugs with the gene info popup while dragging
- ran into the same fatal errors as last time while trying to implement showing the active gene location on the chromosome, so had to hardcode the chromosomes in the component function again
…ser to open gene info popup, this fixed the double clicking glitch on the genelist
implmented load gene function
tried implementing showing the location of activegene on the chromosome,but ran into promise errors for some reason
…ome ISSUES: it mvoes wwith the screen as tyou pan through the chromosomes

PREVIOUS COMMIT: forgot to mention that I Added load selected gene into collections feature, but it has an issue where it only updates the geneticElements state when searching from the searchbar, but the feature still works
changed the zoom in/ out buttons on the map interaction css component(zoom/pan)
…romosome

TODO:
- make it so it shows all the genes in the collection
- style gene info popup
- add logic to determine if gene arrow will be on left or right of chromosome based on strand being + or -
- fix gene list popup placement
Copy link
Collaborator

@Yukthiw Yukthiw left a comment

Choose a reason for hiding this comment

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

Left a few comments largely related to updating the state of the gene list, will do a more formal review after you're finished with this.

Eplant/views/ChromosomeViewer/Viewer/GeneList.tsx Outdated Show resolved Hide resolved
Eplant/views/ChromosomeViewer/Viewer/GeneList.tsx Outdated Show resolved Hide resolved
Eplant/views/ChromosomeViewer/Viewer/GeneList.tsx Outdated Show resolved Hide resolved
Eplant/views/ChromosomeViewer/Viewer/GeneList.tsx Outdated Show resolved Hide resolved
Eplant/views/ChromosomeViewer/Viewer/GeneList.tsx Outdated Show resolved Hide resolved
y330 added 12 commits July 5, 2024 12:30
…ene in collection, but was creating duplicate indicators and had bug so its commented out

- fixed the radius of the small centormeric layers on chromsoome 3
- still has some bugs though
- changed the placement of the text and lines so that its dyanamic, based on the scale, to mirror eplant 2 functionality
- TODO:
add code comments where needed and clean up code
implement ESLint
-added selectedGeneHistory to jotai state to make multiple geneInfoPopups be possible - currently not working
-changed name of geneIndicator to geneAnnotation
- dynamic gap spacing between chromosomes
- dynamic border radius of chromosomes
switched mapinteractioncss library to react-zoomable-ui library for zoom, to better match zoom functionilty of original eplant site
	- combined reset button and scale indicator text
- streamlined activeGeneAnnotation into the main loop to draw all geneAnnotations
- removed heatmap action
- cleaned up state imports in some files
… the tooltips

- Chromosome: added logic when drawing GeneAnnotations that checks if the geneAnnotation object actually corrosponds to an gene in a collection
	+ this is to prevent GeneAnnotations from staying on the screen after the corrosponding gene
	has been removed from the sidebar
- got rid of activeGeneAnnotation prop
- fixed bug where gene annotations dont load when component is refreshed, now they do but it takes a few seconds
fixed gene annotations not being too far away from the chromosome when very zoomed in
fixed snackbar placement
@Yukthiw Yukthiw requested review from zkdan and mwkyuen July 18, 2024 20:48
Copy link
Collaborator

@Yukthiw Yukthiw 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 Yonah! Thanks for doing all of this work. A small comments, largely to do with how state is managed. I noticed thing related to the y-coordinate of the geneAnnotation lines but hopefully this should be a relatively straightforward fix.

I primarily looked at logic so if @zkdan you could take a look at some of the styling related changes that would be great (although it looks good to me).

Eplant/views/ChromosomeViewer/Viewer/GeneInfoPopup.tsx Outdated Show resolved Hide resolved
Eplant/main.tsx Outdated Show resolved Hide resolved
Eplant/UI/LeftNav/Collections.tsx Outdated Show resolved Hide resolved
Eplant/UI/LeftNav/Collections.tsx Outdated Show resolved Hide resolved
Eplant/views/ChromosomeViewer/Viewer/GeneList.tsx Outdated Show resolved Hide resolved
Eplant/views/ChromosomeViewer/index.tsx Outdated Show resolved Hide resolved
Eplant/views/ChromosomeViewer/index.tsx Outdated Show resolved Hide resolved
Eplant/views/ChromosomeViewer/Viewer/GeneAnnotation.tsx Outdated Show resolved Hide resolved
@Yukthiw
Copy link
Collaborator

Yukthiw commented Jul 31, 2024

@y330 Thanks for addressing most of the comments, just a couple things before we get to merging this.

If you could move the helper functions in Chromosome.tsx to another file ("utils.tsx" or whatever) that would help with readability a lot.

I made a comment about how we are handling the fetching of gene data, right now we are making a ton of api requests every time a new gene is added. Cutting down on these should improve performance by a lot but I think this can be left for another PR, just leave the comment there for reference.

Finally, before merging run npx prettier Eplant --write and push the changes to fix formatting issues (in the future adding the format on save feature to your vscode makes things easy)

@y330
Copy link
Collaborator Author

y330 commented Aug 1, 2024

@Yukthiw I moved the helper functions to a new file, and ran npx prettier Eplant --write. I am still not a contributor yet, so i cannot merge the PR

Copy link
Collaborator

@Yukthiw Yukthiw left a comment

Choose a reason for hiding this comment

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

LGTM, thanks Yonah!

@y330 y330 merged commit 93f1060 into BioAnalyticResource:staging Aug 8, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants