-
Notifications
You must be signed in to change notification settings - Fork 270
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
Give option to list unselected cases in the Custom Selection component #2936
Conversation
if (selectMode === SelectMode.SELECTED) { | ||
selectedCases = this.props.selectedSamples; | ||
} else { | ||
const _selectedCaseIds = _.reduce(this.props.selectedSamples, (acc, next) => { |
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.
may be _.keyBy(this.props.selectedSamples,sample=>sample.uniqueSampleKey)
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.
@kalletlak I can update. But do they have same performance? Or just for convenience?
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.
just a cleaner way
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.
See comment with screenshot
@alisman very similar. fontawesome does not have the exact icon in your screenshot |
85e8cce
to
c466c79
Compare
Signed-off-by: Hongxin Zhang <[email protected]>
@zhx828 @alisman @kalletlak it seems like this PR didn't go through product review? Let's discuss on engineering meeting how we can avoid accidentally merging things that still need review |
This solves cBioPortal/cbioportal#6361
This is an improvement pr of #2589