Add tooltips to the phylo tree for both NCBI and Sample entries #1532
Conversation
* New DataTooltip container component for generic data tooltips * Links to NCBI and samples page * Save session state to allow refreshing same page
|
Code Climate has analyzed commit df6cb2f and detected 0 issues on this pull request. View more on Code Climate. |
| return urlParams.treeId || (phyloTrees[0] || {}).id; | ||
| return ( | ||
| urlParams.treeId || | ||
| parseInt(window.sessionStorage.getItem("treeId")) || |
There was a problem hiding this comment.
did we all agree that window.sessionStorage is OK? is it supported on all the modern browsers
There was a problem hiding this comment.
Should be fine: https://developer.mozilla.org/en-US/docs/Web/API/Window/sessionStorage. We should do a more through css/js audit for issues in older browsers.
| handleNodeClick(node) { | ||
| if (node.data.accession) { | ||
| let url = `https://www.ncbi.nlm.nih.gov/nuccore/${node.data.accession}`; | ||
| window.open(url, "_blank"); |
There was a problem hiding this comment.
Add rel="noopener noreferrer" to all external links https://www.jitbit.com/alexblog/256-targetblank---the-most-underestimated-vulnerability-ever/
There was a problem hiding this comment.
| this.sampleFields = [ | ||
| { name: "project_name", label: "Project" }, | ||
| { name: "sample_location", label: "Location" }, | ||
| { name: "created_at", label: "Upload Date" }, |
There was a problem hiding this comment.
ideally this would be displayed using <Moment />
There was a problem hiding this comment.
Good catch. I thought of that when I was adding the field, but forgot to add. Done.
| { name: "sample_library", label: "Library" }, | ||
| { name: "sample_sequencer", label: "Sequencer" }, | ||
| { name: "sample_unique_id", label: "Unique ID" }, | ||
| { name: "sample_notes", label: "Notes" } |
There was a problem hiding this comment.
I'm sorry I realized I forgot some:
t.float "sample_input_pg", limit: 24 # RNA/DNA input (pg)
t.integer "sample_batch" # Batch
t.text "sample_diagnosis" # Clinical diagnosis
t.string "sample_detection" # Detection method
There was a problem hiding this comment.
Looks really great. There are some weird issues when the popup is too big for the window (it quickly jumps back and forth rather than staying in place and the window becoming scrollable). Not sure if anything can be done about that in the timeframe of the sprint.
Use moment for upload date.
…web into tiago/sprint-phylo
|
@cdebourcy I think that can be addressed during stabilization. It might take me a while to figure out a good solution for that. Scrolling would not work because as you scroll, you probably lose the tooltip. |
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change. Explain both the expected functionality and the edge cases.
Type of change