-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
ui: fix db page Node/Regions column rendering #105765
Conversation
Previously, the db page was not updating its columns if the `showNodeRegionsColumn` prop changed. The db details page was also not filtering out the regions column when desired. Epic: none Release note (bug fix): node/regions columns in db and db details page should properly render. This column is hidden for tenants and otherwise is shown for clusters with > 1 node.
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.
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @xinhaoz)
pkg/ui/workspaces/cluster-ui/src/databasesPage/databasesPage.tsx
line 289 at r1 (raw file):
prevProps.indexRecommendationsEnabled !== this.props.indexRecommendationsEnabled || prevProps.showNodeRegionsColumn !== this.props.showNodeRegionsColumn
In principle, the change here is saying we should render this column if the # of nodes or regions changes to >1. Was this the case that was causing the cypress test to fail?
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.
oops meant to approve
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @xinhaoz)
This bit was the part causing tests to fail, yes, but the prop was always based on if we had more than 1 node. The issue here was that we weren't recomputing the columns when this prop changed -- in the cypress testing it just so happens that this is the first page we request the nodes info, so the prop flips when the request returns. |
TFTR! Test failure unrelated to UI. |
Build succeeded: |
Previously, the db page was not updating its columns if the
showNodeRegionsColumn
prop changed. The db details page wasalso not filtering out the regions column when desired.
Epic: none
Release note (bug fix): node/regions columns in db and db details
page should properly render. This column is hidden for tenants and
otherwise is shown for clusters with > 1 node.