-
Notifications
You must be signed in to change notification settings - Fork 247
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
Add tooltip for annotation column #3826
Conversation
0d28d49
to
ed2b3a7
Compare
@@ -231,6 +231,12 @@ export function getAnnotationData( | |||
oncoKbIndicator, | |||
oncoKbAvailableDataTypes, | |||
}; | |||
} else if (oncoKbData && oncoKbData.isPending) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add this because we don't catch pending status for oncoKbData
correctly. If oncoKbGeneExist
is false then it directly goes to
else {
value = {
...value,
oncoKbStatus: 'complete',
oncoKbIndicator: undefined,
};
}
But at this moment oncoKbData
could still be pending
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for putting this together! Great work!
There are some more show/enable properties for each resource that would be good to handle
Try e.g.
localStorage.frontendConfig = '{"serverConfig":{"show_civic":false}}'
It will still show the civic icon in the header (prolly good to not show in the header if we've disabled the header at the app configuration level):
Similarly for other resources it would be good to not show them in the header if they are disabled (oncokb, hotspots, civic, mycancergenome)
TODO for testing:
- patient view
- mutation mapper tool
486fa01
to
b609cf2
Compare
[OncokbTherapeuticLevelIconEnum.LEVEL_1]: | ||
'FDA-recognized biomarker predictive of response to an FDA-approved drug in this indication', | ||
[OncokbTherapeuticLevelIconEnum.LEVEL_2]: | ||
'Standard care biomarker recommended by the NCCN or other expert panels predictive of response to an FDA-approved drug in this indication', | ||
[OncokbTherapeuticLevelIconEnum.LEVEL_3A]: | ||
'Compelling clinical evidence supports the biomarker as being predictive of response to a drug in this indication', | ||
[OncokbTherapeuticLevelIconEnum.LEVEL_3B]: | ||
'Standard care or investigational biomarker predictive of response to an FDA-approved or investigational drug in another indication', | ||
[OncokbTherapeuticLevelIconEnum.LEVEL_4]: | ||
'Compelling biological evidence supports the biomarker as being predictive of response to a drug', | ||
[OncokbTherapeuticLevelIconEnum.LEVEL_R1]: | ||
'Standard care biomarker predictive of resistance to an FDA-approved drug in this indication', | ||
[OncokbTherapeuticLevelIconEnum.LEVEL_R2]: | ||
'Compelling clinical evidence supports the biomarker as being predictive of resistance to a drug', | ||
[OncokbDiagnosticLevelIconEnum.LEVEL_DX1]: | ||
'FDA and/or professional guideline-recognized biomarker required for diagnosis in this indication', | ||
[OncokbDiagnosticLevelIconEnum.LEVEL_DX2]: | ||
'FDA and/or professional guideline-recognized biomarker that supports diagnosis in this indication', | ||
[OncokbDiagnosticLevelIconEnum.LEVEL_DX3]: | ||
'Biomarker that may assist disease diagnosis in this indication based on clinical evidence', | ||
[OncokbPrognosticLevelIconEnum.LEVEL_PX1]: | ||
'FDA and/or professional guideline-recognized biomarker prognostic in this indication based on well-powered studie(s)', | ||
[OncokbPrognosticLevelIconEnum.LEVEL_PX2]: | ||
'FDA and/or professional guideline-recognized biomarker prognostic in this indication based on a single or multiple small studies', | ||
[OncokbPrognosticLevelIconEnum.LEVEL_PX3]: | ||
'Biomarker is prognostic in this indication based on clinical evidence in well-powered studies', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In react-mutation-mapper, we have something similar to this. @onursumer I also realize there are good amount of content here can be shared between RMM and portal. What do you suggest in terms of how to resolve the dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably get these from react-mutation-mapper
if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Just a few minor suggestions.
if ( | ||
document | ||
.getElementById('sv-annotation') | ||
?.getElementsByClassName('oncokb-content')[0]?.clientWidth | ||
) { | ||
this.oncokbWidth = | ||
Number( | ||
document | ||
.getElementById('sv-annotation') | ||
?.getElementsByClassName('oncokb-content')[0] | ||
?.clientWidth | ||
) || 22; | ||
clearInterval(this.oncokbInterval); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we extract this to a utility function? this is very similar to the one for copy-number-annotation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@onursumer It could work but would it introduce some other code? Because we need to have clearInterval(this.oncokbInterval);
, so for example if we move them to a util function, then we will have something like this:
this.oncokbInterval = setInterval(() => {
someUtilFunction(this.setOncokbWidth, this.clearIntervalFn);
}, 500);
and we will have to add this wherever that util function is called:
@action.bound
private clearIntervalFn() {
clearInterval(this.oncokbInterval);
}
@action.bound
private setOncokbWidth(width: number) {
this.oncokbWidth = width;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not worry about this then for now.
[OncokbTherapeuticLevelIconEnum.LEVEL_1]: | ||
'FDA-recognized biomarker predictive of response to an FDA-approved drug in this indication', | ||
[OncokbTherapeuticLevelIconEnum.LEVEL_2]: | ||
'Standard care biomarker recommended by the NCCN or other expert panels predictive of response to an FDA-approved drug in this indication', | ||
[OncokbTherapeuticLevelIconEnum.LEVEL_3A]: | ||
'Compelling clinical evidence supports the biomarker as being predictive of response to a drug in this indication', | ||
[OncokbTherapeuticLevelIconEnum.LEVEL_3B]: | ||
'Standard care or investigational biomarker predictive of response to an FDA-approved or investigational drug in another indication', | ||
[OncokbTherapeuticLevelIconEnum.LEVEL_4]: | ||
'Compelling biological evidence supports the biomarker as being predictive of response to a drug', | ||
[OncokbTherapeuticLevelIconEnum.LEVEL_R1]: | ||
'Standard care biomarker predictive of resistance to an FDA-approved drug in this indication', | ||
[OncokbTherapeuticLevelIconEnum.LEVEL_R2]: | ||
'Compelling clinical evidence supports the biomarker as being predictive of resistance to a drug', | ||
[OncokbDiagnosticLevelIconEnum.LEVEL_DX1]: | ||
'FDA and/or professional guideline-recognized biomarker required for diagnosis in this indication', | ||
[OncokbDiagnosticLevelIconEnum.LEVEL_DX2]: | ||
'FDA and/or professional guideline-recognized biomarker that supports diagnosis in this indication', | ||
[OncokbDiagnosticLevelIconEnum.LEVEL_DX3]: | ||
'Biomarker that may assist disease diagnosis in this indication based on clinical evidence', | ||
[OncokbPrognosticLevelIconEnum.LEVEL_PX1]: | ||
'FDA and/or professional guideline-recognized biomarker prognostic in this indication based on well-powered studie(s)', | ||
[OncokbPrognosticLevelIconEnum.LEVEL_PX2]: | ||
'FDA and/or professional guideline-recognized biomarker prognostic in this indication based on a single or multiple small studies', | ||
[OncokbPrognosticLevelIconEnum.LEVEL_PX3]: | ||
'Biomarker is prognostic in this indication based on clinical evidence in well-powered studies', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably get these from react-mutation-mapper
if possible.
5805b2a
to
f1d3870
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
793aa26
to
b8f1b9f
Compare
62633fd
to
74c9205
Compare
Fix: cBioPortal/cbioportal#8305
Part of: cBioPortal/cbioportal#8711
Test: query any study and check mutations tab, patients view and mutation mapper tool
For example:
https://deploy-preview-3826--cbioportalfrontend.netlify.app/results/mutations?cancer_study_list=msk_impact_2017&Z_SCORE_THRESHOLD=2.0&RPPA_SCORE_THRESHOLD=2.0&profileFilter=mutations%2Cfusion%2Ccna&case_set_id=msk_impact_2017_cnaseq&gene_list=BRAF&geneset_list=%20&tab_index=tab_visualize&Action=Submit