Query Browser: Make SeriesButton key always unique #2495
Conversation
openshift-ci-robot
added
size/XS
|
/retest |
1 similar comment
|
/retest |
| @@ -518,7 +518,7 @@ const QueryTable_: React.FC<QueryTableProps> = ({index, isEnabled, isExpanded, q | |||
| rowMapper = ({metric, value}) => [ | |||
| // TableBody's shouldComponentUpdate seems to struggle with SeriesButton, so add a unique key to help TableBody | |||
| // determine when it should update | |||
| {title: <SeriesButton index={index} key={JSON.stringify([index, metric])} labels={metric} />}, | |||
| {title: <SeriesButton index={index} key={_.uniqueId()} labels={metric} />}, | |||
There was a problem hiding this comment.
Is this a bug with the PF4 table?
cc @priley86
There was a problem hiding this comment.
@priley86 For context, we're seeing cases where the table isn't re-rending. This is a hack to make sure it always re-renders, but I'd like to track this down further if it might be a problem in Patternfly.
There was a problem hiding this comment.
hey @spadgett - was having a conversation w/ Andy about this yesterday. Here's my current thoughts...
you are correct that you would need to pass a prop on the row to force re-rendering to occur. This is sort of a “blind optimization” on our part - we check if any row props have changed to try to prevent all rows from re-rendering : https://github.com/patternfly/patternfly-react/blob/547c644e68fb9f36d068aab9427a7ef533ed1cf9/packages/patternfly-4/react-table/src/components/Table/base/body-row.tsx#L44
You can override the row’s shouldComponentUpdate method by using a RowWrapper like this on your Table definition: https://reactabular.js.org/#/table/overriding-should-component-update
Or you can just provide the key with unique ID like you have it (causing re-render of children to occur each time).
If you do not feel like this resolves it, please feel free to suggest an alternative though.
There was a problem hiding this comment.
Basically, this is an attempt to optimize row rendering. If you think it a poor optimization, it can certainly be modified or removed. Let me know!
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kyoto, spadgett The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Unfortunately,
[index, metric]appears to not be unique enough, causing the UI to occasionally seize up when rendering the query results table.\cc @spadgett