Skip to content
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 badges for filter elements #98988

Merged
merged 1 commit into from
Mar 21, 2023
Merged

Conversation

maryliag
Copy link
Contributor

@maryliag maryliag commented Mar 19, 2023

Adds badges for each of the selected filters on SQL Activity and
Insights pages.

Part Of #98891

https://www.loom.com/share/e7417209fc704d71b2733f58609fb4de

Screenshot 2023-03-19 at 1 30 49 PM

Release note (ui change): Adds badges for each selected
filter on SQL Activity and Insights pages.

@maryliag maryliag requested review from a team March 19, 2023 17:33
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@THardy98 THardy98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 4 files at r1, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @dongniwang and @maryliag)


pkg/ui/workspaces/cluster-ui/src/queryFilter/filter.tsx line 814 at r1 (raw file):

  const badges = Object.keys(filters).map(filter => {
    if (
      filters[filter] != null &&

Not a big deal, but I see we do this check in calculateActiveFilters in the statements page as well, except it returns the number of active filters.

Maybe we could extend that to return the filters that are active, and in places that need the number we return the length?

Copy link
Contributor Author

@maryliag maryliag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @dongniwang and @THardy98)


pkg/ui/workspaces/cluster-ui/src/queryFilter/filter.tsx line 814 at r1 (raw file):

Previously, THardy98 (Thomas Hardy) wrote…

Not a big deal, but I see we do this check in calculateActiveFilters in the statements page as well, except it returns the number of active filters.

Maybe we could extend that to return the filters that are active, and in places that need the number we return the length?

Done

Copy link
Contributor

@THardy98 THardy98 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 1 of 4 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @dongniwang)

@dongniwang
Copy link

LGTM! Thanks for the quick turnaround!

Copy link

@dongniwang dongniwang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@maryliag maryliag force-pushed the filter-selection branch 2 times, most recently from eeca861 to 9e9c89c Compare March 21, 2023 03:24
Adds badges for each of the selected filters on SQL Activity and
Insights pages.
Part Of cockroachdb#98891

Release note (ui change): Adds badges for each selected
filter on SQL Activity and Insights pages.
@maryliag
Copy link
Contributor Author

TFTR!
bors r+

@craig
Copy link
Contributor

craig bot commented Mar 21, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Mar 21, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Mar 21, 2023

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants