-
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: add column selector to transation page #70262
Conversation
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 11 of 11 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @maryliag)
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 11 files at r1.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @maryliag)
pkg/ui/workspaces/cluster-ui/src/statsTableUtil/statsTableUtil.tsx, line 41 at r1 (raw file):
rowsRead: "Rows Read", statements: "Statements", statementsCount: "Statements",
statements
and statementsCount
have the same column label ?
pkg/ui/workspaces/cluster-ui/src/statsTableUtil/statsTableUtil.tsx, line 588 at r1 (raw file):
); }, statementsCount: (statType: StatisticType) => {
Do we also want to annotate the return types here like we did for other places in this PR ?
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.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @Azhng)
pkg/ui/workspaces/cluster-ui/src/statsTableUtil/statsTableUtil.tsx, line 41 at r1 (raw file):
Previously, Azhng (Archer Zhang) wrote…
statements
andstatementsCount
have the same column label ?
same column label, different tooltip. That's why they need to be different keys here
pkg/ui/workspaces/cluster-ui/src/statsTableUtil/statsTableUtil.tsx, line 588 at r1 (raw file):
Previously, Azhng (Archer Zhang) wrote…
Do we also want to annotate the return types here like we did for other places in this PR ?
the return type here is a little trickier, we're returning as Element, but is we add return type Element it requires several other parameters, so I made the decision of leaving those one as is
Add column selector to Transaction Page Fixes cockroachdb#70148 Release justification: Category 4 Release note (ui change): Add column selector to transaction page
926ef9c
to
662c466
Compare
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.
Reviewable status: complete! 1 of 0 LGTMs obtained (and 1 stale) (waiting on @maryliag)
bors r+ |
Build succeeded: |
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from 662c466 to blathers/backport-release-21.2-70262: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 21.2.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
Add column selector to Transaction Page
Fixes #70148
Release justification: Category 4
Release note (ui change): Add column selector to transaction page