Add plots tab to study view #4802
Conversation
gblaih
force-pushed
the
study-plots-tab
branch
from
d7cb940 to
5fcc3c0
Compare
✅ Deploy Preview for cbioportalfrontend ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
gblaih
force-pushed
the
study-plots-tab
branch
from
1f3b315 to
9965332
Compare
gblaih
force-pushed
the
study-plots-tab
branch
2 times, most recently
from
3c8a37c to
2af58eb
Compare
| @@ -0,0 +1,5996 @@ | |||
| import * as React from 'react'; | |||
There was a problem hiding this comment.
This is based on PlotsTab, right? It is virtually impossible to review the changes here because of ~6000 new lines. Do we still plan to keep the old PlotsTab component, or are we eventually going to replace it with this one?
Also, not sure if PlotsFeature is a good name for a component. It sounds a bit strange to have Feature in a component's name.
There was a problem hiding this comment.
yep, sorry about that. PlotsTab has been replaced with what was PlotsFeature
| @@ -78,6 +78,7 @@ import { | |||
| prepareCustomTabConfigurations, | |||
| } from 'shared/lib/customTabs/customTabHelpers'; | |||
| import { VirtualStudyModal } from 'pages/studyView/virtualStudy/VirtualStudyModal'; | |||
| import PlotsFeature from 'pages/resultsView/plots/PlotsFeature'; | |||
There was a problem hiding this comment.
Ideally study view should not import components from result view. We should probably move the PlotsFeature component under shared.
| }, | ||
| }); | ||
|
|
||
| public groupSamplesByCancerType( |
There was a problem hiding this comment.
This seems like an almost duplicate of
It would be great if we could define utility functions for these kind of duplicates to maximize code reuse.
| }, | ||
| }); | ||
|
|
||
| readonly structuralVariants = remoteData<StructuralVariant[]>({ |
There was a problem hiding this comment.
This is another relatively long function duplicated from ResultsViewPageStore
| ); | ||
| } | ||
|
|
||
| readonly samplesSpecification = remoteData({ |
There was a problem hiding this comment.
Another relatively long function duplicated from ResultsViewPageStore. Would be nice to identify all those duplicate functions and write a generalized utility function for each if possible.
| return { | ||
| value: geneOptions[0].value, | ||
| label: self.isGeneProfiled(geneOptions[0].label) | ||
| ? geneOptions[0].label | ||
| : geneOptions[0].label + ' (Not profiled)', | ||
| }; |
There was a problem hiding this comment.
Not a big deal but you can write a getOption(option) function for this code segment. We do the same thing for two other cases as well.
| @@ -1832,7 +2053,8 @@ export default class PlotsTab extends React.Component<IPlotsTabProps, {}> { | |||
| // add gene options | |||
| allOptions.push({ | |||
| label: 'Genes', | |||
| options: this.props.store.genes.result!.map(gene => ({ | |||
| // options: this.props.genes.result!.filter(g => g.hugoGeneSymbol === 'KRAS').map(gene => ({ | |||
| if (coverageInformation) { | ||
| return Object.values(coverageInformation.samples).some( | ||
| s => !_.isEmpty(s.allGenes) || !!s.byGene[gene] | ||
| ); | ||
| } | ||
| if (this.props.coverageInformation.isComplete) { | ||
| return Object.values( | ||
| this.props.coverageInformation.result!.samples | ||
| ).some(s => !_.isEmpty(s.allGenes) || !!s.byGene[gene]); | ||
| } |
There was a problem hiding this comment.
something like this would be better I think
| if (coverageInformation) { | |
| return Object.values(coverageInformation.samples).some( | |
| s => !_.isEmpty(s.allGenes) || !!s.byGene[gene] | |
| ); | |
| } | |
| if (this.props.coverageInformation.isComplete) { | |
| return Object.values( | |
| this.props.coverageInformation.result!.samples | |
| ).some(s => !_.isEmpty(s.allGenes) || !!s.byGene[gene]); | |
| } | |
| const samples = coverageInformation?.samples || this.props.coverageInformation.result?.samples; | |
| if (samples) { | |
| return Object.values(samples).some( | |
| s => !_.isEmpty(s.allGenes) || !!s.byGene[gene] | |
| ); | |
| } |
| @@ -4761,6 +5071,33 @@ export default class PlotsTab extends React.Component<IPlotsTabProps, {}> { | |||
| ); | |||
| } | |||
|
|
|||
| @computed get showNoDataTypesSelectedWarning() { | |||
There was a problem hiding this comment.
this function returns a boolean
| @computed get showNoDataTypesSelectedWarning() { | |
| @computed get shouldShowNoDataTypesSelectedWarning() { |
| ); | ||
| } | ||
|
|
||
| @computed get showNoGenesSelectedWarning() { |
There was a problem hiding this comment.
same here
| @computed get showNoGenesSelectedWarning() { | |
| @computed get shouldShowNoGenesSelectedWarning() { |
…data type options
Implements cBioPortal/cbioportal#10443