-
Notifications
You must be signed in to change notification settings - Fork 271
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
On Study View set showNA to false by default for histogram #4876
Conversation
✅ Deploy Preview for cbioportalfrontend ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Found issue with Bar charts loading. Try fix now |
@inodb can you QA this and merge? |
Hello! @alisman I found the bug in this code. Please do not merge this PR. This somehow breaks the Bar Chart view, where there is no NA toggle available. For example for this case: Adding the code in PR, would completely erase the view of the data: I spend some time to understand why this happens, but i couldn't find the reason. Can someone help look into this issue? Thank you! |
I fixed the issue with the Bar Charts. The problems was with the conditions in the get labelShowingNA() function The reason for this was, because the showNAChecked is now false by default, but in the tickValuesBasedOnNA() function if (this.props.showNAChecked) -> the code will enter this block and this line will completely erase the actual numerical values. Therefore, we need to introduce the additional if guards to the functions to handle empty conditions. Thank you! |
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.
LGTM! Just screenshots need to be updated
7701469
to
1b90a54
Compare
Fix cBioPortal/cbioportal#10661 (see https://help.github.com/en/articles/closing-issues-using-keywords)
Describe changes proposed in this pull request:
Checks
Any screenshots or GIFs?
No
Notify reviewers
@alisman @inodb
Read our Pull request merging
policy. It can help to figure out who worked on the
file before you. Please use
git blame <filename>
to determine thatand notify them either through slack or by assigning them as a reviewer on the PR