Add driver annotation source selector and maintain track visibility in comparison view lollipop #4463
Conversation
…switching between genes
gblaih
force-pushed
the
group-comparison-lollipop-plot-annotation-source
branch
from
f7a3f7f to
c293c94
Compare
| </span> | ||
| } | ||
| /> | ||
| {!this.props.comparisonView && ( |
There was a problem hiding this comment.
Ideally this component shouldn't have view specific props like resultsView, comparisonView, etc. Instead we should have generic props like disableInfoIcon, etc. Also, now we use the component in multiple views, but it is still under src/pages/resultsView. Ideally we should also relocate it to a shared location.
Probably better to address these issues in a separate PR.
There was a problem hiding this comment.
Agreed. Unless someone asked for this specifically, I think this can be handled outside, next to the button/text that opens the menu. In fact, since the button is quite specific to the tab (not in header) it may be implicit that it only applies to this tab.
There was a problem hiding this comment.
SettingsMenu will have a prop titleComponent
you just pass this jsx
| <SettingsMenuButton | ||
| store={this.props.store} | ||
| comparisonView={true} | ||
| /> |
There was a problem hiding this comment.
Is this the best location for this button? It kind of looks like a button to change the frequency settings.
There was a problem hiding this comment.
Agreed. I put a suggestion in general comment. Lets discuss with product next week.
| @@ -107,6 +111,11 @@ export default class GroupComparisonMutationsTabPlot extends React.Component< | |||
| } | |||
| } | |||
|
|
|||
| @action.bound | |||
| protected onClickSettingMenu(visible: boolean) { | |||
There was a problem hiding this comment.
i don't understand why the plot needs to know about settings menu? it seems like a concern of the parent tab component and indeed that's where the settings button is.
There was a problem hiding this comment.
Actually we have multiple settings buttons. There is one hidden in the filter panel, and the same button appears in the tooltip as well. So, 3 buttons for the same functionality :)
The component name plot is a bit confusing here. This component is not just the lollipop plot, it also contains the filter panel, 3D visualzier, etc. It's basically still a MutationMapper component with the table disabled.
| @@ -93,7 +94,7 @@ export default class DriverAnnotationControls extends React.Component< | |||
| </label> | |||
| </div> | |||
| <div style={{ marginLeft: '20px' }}> | |||
| {this.props.resultsView && ( | |||
| {(this.props.resultsView || this.props.comparisonView) && ( | |||
There was a problem hiding this comment.
change this to showOnckbAnnotationControls instead of resultsView and comparisonView
Adds features in cBioPortal/cbioportal#9901