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

Capture selected scale in URL in group comparison lollipop plot #4436

Merged

Conversation

gblaih
Copy link
Contributor

@gblaih gblaih commented Dec 1, 2022

Fixes issue in cBioPortal/cbioportal#9901

  • Makes scale toggle icons clearer and captures state in URL

@gblaih gblaih force-pushed the group-comparison-lollipop-plot-scale-url branch from 334f741 to 55efa03 Compare December 1, 2022 16:32
@gblaih gblaih requested a review from alisman December 1, 2022 19:55
@gblaih gblaih force-pushed the group-comparison-lollipop-plot-scale-url branch from a3306c1 to e9d6696 Compare December 1, 2022 20:30
@gblaih gblaih force-pushed the group-comparison-lollipop-plot-scale-url branch from e9d6696 to b58c35b Compare December 2, 2022 16:17
@@ -132,6 +132,7 @@ export default class GroupComparisonMutationsTab extends React.Component<
</div>
<GroupComparisonMutationsTabPlot
store={this.props.store}
urlWrapper={this.props.urlWrapper}
Copy link
Collaborator

Choose a reason for hiding this comment

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

it would be better to give this component a property onScaleToggle and then update the urlWrapper at this level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed, gave component onScaleToggle

@@ -130,7 +132,11 @@ export default class GroupComparisonMutationsTabPlot extends React.Component<
plotLollipopTooltipCountInfo={
plotLollipopTooltipCountInfo
}
axisMode={this.axisMode}
axisMode={
Copy link
Collaborator

Choose a reason for hiding this comment

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

again, just give the component an axisMode property and keep it agnostic to the url, which is really a concert of the page component

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@gblaih gblaih force-pushed the group-comparison-lollipop-plot-scale-url branch from a6fcd70 to f21eef6 Compare December 2, 2022 23:00
@gblaih gblaih force-pushed the group-comparison-lollipop-plot-scale-url branch from f21eef6 to 32b8665 Compare December 5, 2022 16:16
@alisman alisman merged commit 6f47580 into cBioPortal:master Dec 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants