Add Categorical Bar Chart on Study View #4951
Conversation
✅ Deploy Preview for cbioportalfrontend ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
olzhasmukayev
force-pushed
the
feat/category-bar-chart
branch
from
8f83fcc
to
c4893d4
Compare
| @@ -312,3 +324,18 @@ export const STUDY_VIEW_CONFIG: StudyViewConfig = _.assign( | |||
| studyViewFrontEnd, | |||
| (getServerConfig() || {}).study_view | |||
| ); | |||
|
|
|||
| export const chartChangeOptionsMap: ChangeChartOptionsMap = { | |||
| uniqueKey: string, | ||
| data: ClinicalDataCount[] | string[] | ||
| ): void { | ||
| const { totalCount, naCount } = (data as ( |
There was a problem hiding this comment.
add comments here to explain what's happening here. can also add comments below to describe the method of choosing which chart
| @@ -414,6 +416,85 @@ export class ChartHeader extends React.Component<IChartHeaderProps, {}> { | |||
| ); | |||
| } | |||
|
|
|||
| if ( | |||
| this.props.chartControls && | |||
There was a problem hiding this comment.
can simplify to this.props.chartControls?.showChartChangeOptions
|
|
||
| @autobind | ||
| private onSelection(bars: { data: BarDatum[] }[]): void { | ||
| const dataBins = _.flatten( |
There was a problem hiding this comment.
when performing multiple array functions, lets chain multiple lodash functions like
_(bars) .function() .function() ...
lodash has a map function
| const TILT_ANGLE = 30; | ||
|
|
||
| @observer | ||
| export default class CategoryBarChart |
There was a problem hiding this comment.
i'm noticing some similarities with our existing BarChart component. can we look into maybe extending it or reusing that code to prevent duplicate code?
| } | ||
| } | ||
|
|
||
| const onlyContainsNA = (element: BarDatum): boolean => { |
There was a problem hiding this comment.
like said above, these functions are in BarChart as well. we could make export these functions to prevent duplication
| import { VictoryLabel, Helpers } from 'victory'; | ||
| import { Tooltip, OverlayTrigger } from 'react-bootstrap'; | ||
|
|
||
| export class CategoryBarChartAxisLabel extends VictoryLabel { |
There was a problem hiding this comment.
same as above. i think we have very similar components with our BarChart component. it would be nice to reuse this somehow. lets look into this for all similarly related components
| updateCurrentBinsValue() { | ||
| let newBins: number[] = []; | ||
| if (this.currentBinMethod === BinMethodOption.CUSTOM) { | ||
| newBins = _.sortBy( |
There was a problem hiding this comment.
lodash chaining here as well
| @@ -1451,6 +1484,11 @@ export class ChartContainer extends React.Component<IChartContainerProps, {}> { | |||
| () => (this.newlyAdded = false), | |||
| STUDY_VIEW_CONFIG.thresholds.chartHighlight | |||
| ); | |||
| } else if (this.chartChanged) { | |||
There was a problem hiding this comment.
currently when changing a chart type, the highlight doesn't go away. look into the mechanism behind this and figure out why
inodb
changed the title
|
@olzhasmukayev Nice work! Can we enable it for Metastatic Sites as well? 
|
olzhasmukayev
force-pushed
the
feat/category-bar-chart
branch
from
c4893d4
to
1e659a4
Compare
olzhasmukayev
force-pushed
the
feat/category-bar-chart
branch
from
66fcf9a
to
4a7e78b
Compare
olzhasmukayev
force-pushed
the
feat/category-bar-chart
branch
from
b3116f6
to
dcf08bf
Compare
Choose chart type: