ui: search criteria ux improvements#100893
Conversation
|
This change is |
9b0ca34 to
20916b4
Compare
gtr
left a comment
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @maryliag)
pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx line 609 at r1 (raw file):
<InlineAlert intent="warning" title={getSubsetWarning(
Missing the new updateSortValue argument.
pkg/ui/workspaces/cluster-ui/src/util/sqlActivityConstants.ts line 97 at r1 (raw file):
export const STATS_LONG_LOADING_DURATION = duration(2, "s"); export function getSubsetWarning(
nit: you might have to rename this file to sqlActivityConstants.tsx since you're using JSX syntax in this function. This will also get rid of the failing lint tests.
pkg/ui/workspaces/cluster-ui/src/util/sqlActivityConstants.ts line 105 at r1 (raw file):
updateSortValue: (sort: SqlStatsSortType) => void, ): string { const link = (
I don't see this link component being used anywhere.
pkg/ui/workspaces/cluster-ui/src/util/sqlActivityConstants.ts line 108 at r1 (raw file):
<a className={"action"} onClick={() => updateSortValue(getLabel(columnTitle, type))}
I'm a little confused about calling this getLabel function. Did you mean to call getSortLabel?
Some of the names of sort on search criteria were not a match for the column name on the tables, which could cause confusion. This commit updates the values of "P99" to "P99 Latency" and "Service Latency" to "Statement time" and "Transaction time". Epic: None Release note (ui change): Update sort label on Search Criteria to match the name on the table columns.
20916b4 to
cbb3491
Compare
maryliag
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/ui/workspaces/cluster-ui/src/transactionsPage/transactionsPage.tsx line 609 at r1 (raw file):
Previously, gtr (Gerardo Torres Castro) wrote…
Missing the new
updateSortValueargument.
That was not part of this commit, removed
pkg/ui/workspaces/cluster-ui/src/util/sqlActivityConstants.ts line 108 at r1 (raw file):
Previously, gtr (Gerardo Torres Castro) wrote…
I'm a little confused about calling this
getLabelfunction. Did you mean to callgetSortLabel?
Ooops... All the 3 changes that you commented on was something I was working on another commit that was not suppose to be in this one!
Removed all of these things, please take another look
gtr
left a comment
There was a problem hiding this comment.
Reviewed 2 of 3 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status:
|
TFTR! |
|
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 cbb3491 to blathers/backport-release-22.2-100893: 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 22.2.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Some of the names of sort on search criteria were
not a match for the column name on the tables, which could cause confusion. This commit updates the values of "P99" to "P99 Latency" and "Service Latency" to "Statement time" and "Transaction time".
Release note (ui change): Update sort label
on Search Criteria to match the name on the table columns.
Epic: CRDB-25476