ref(dashboards): make WidgetCardChart functional
#97885
Conversation
github-actions
bot
added
the
Scope: Frontend
| const sortMatches = | ||
| props.location.query[WidgetViewerQueryField.SORT] === | ||
| prevProps.location.query[WidgetViewerQueryField.SORT]; |
There was a problem hiding this comment.
woops sorry for the late reply, got distracted with other stuff. location isn't a prop anymore, so think we're good here.
| dashboardFilters?: DashboardFilters; | ||
| disableTableActions?: boolean; | ||
| disableZoom?: boolean; | ||
| expandNumbers?: boolean; | ||
| isMobile?: boolean; | ||
| isSampled?: boolean | null; | ||
| legendOptions?: LegendComponentOption; | ||
| minTableColumnWidth?: number; | ||
| noPadding?: boolean; | ||
| onLegendSelectChanged?: EChartEventHandler<{ | ||
| name: string; | ||
| selected: Record<string, boolean>; | ||
| type: 'legendselectchanged'; | ||
| }>; | ||
| onWidgetTableResizeColumn?: (columns: TabularColumn[]) => void; | ||
| onWidgetTableSort?: (sort: Sort) => void; | ||
| onZoom?: EChartDataZoomHandler; | ||
| router?: InjectedRouter; |
There was a problem hiding this comment.
I'm assuming that after the refactor it turned out that these weren't used?
There was a problem hiding this comment.
Yes to router and expandNumbers. I moved the table related props under the TableResultProps. Which now you point out, I should probably rename it to TableComponentProps or the like
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| props.widget.displayType !== DisplayType.TOP_N && !defined(props.widget.limit); | ||
| const legendMatches = isEqual(props.legendOptions, prevProps.legendOptions); | ||
| return selectionMatches && (sortMatches || isNotTopNWidget) && legendMatches; | ||
| return selectionMatches && isNotTopNWidget && legendMatches; |
There was a problem hiding this comment.
Bug: TopN Widgets Suffer Unnecessary Re-renders
The shouldWidgetCardChartMemo logic now causes TopN widgets and widgets with limits to always re-render. This is because isNotTopNWidget is false for them, and the sortMatches condition was removed, resulting in unnecessary re-renders and a performance regression.
### Changes Make `WidgetCardChart` functional. Also doesn't seem like `expandNumbers` is used anymore. I'll look into opening another PR to clean up the props if this one gets merged in
### Changes Make `WidgetCardChart` functional. Also doesn't seem like `expandNumbers` is used anymore. I'll look into opening another PR to clean up the props if this one gets merged in
Changes
Make
WidgetCardChartfunctional.Also doesn't seem like
expandNumbersis used anymore. I'll look into opening another PR to clean up the props if this one gets merged in