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.

Sign up for GitHub

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

Jump to bottom

Navigate filtered checkresults #883

Merged
merged 2 commits into from
Oct 10, 2022
Merged

Navigate filtered checkresults #883

merged 2 commits into from
Oct 10, 2022

Conversation

rtorrero
Copy link
Contributor

Description

This PR allows to navigate from the cluster details overview directly to the check results filtered based on which check result state is clicked. See the attached gif:

filtered-links

How was this tested?

E2E tests should be passing

@rtorrero rtorrero added enhancement New feature or request ux labels Oct 10, 2022
arbulu89
arbulu89 requested changes Oct 10, 2022
View reviewed changes
Copy link
Contributor

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

@rtorrero Great! Just some small comments.
I think we should add a new test here, whether it is a e2e or unit test with jest. Having the current tests passing doesn't cover this new feature

assets/js/components/ClusterDetails/ClusterDetails.jsx Outdated Show resolved Hide resolved
assets/js/components/ClusterDetails/ChecksResultOverview.jsx Show resolved Hide resolved
assets/js/components/ClusterDetails/ChecksResultFilters.jsx Outdated Show resolved Hide resolved
dottorblaster
dottorblaster approved these changes Oct 10, 2022
View reviewed changes
Copy link
Contributor

@dottorblaster dottorblaster left a comment

Choose a reason for hiding this comment

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

I was about to suggest to turn let lets to consts as well, super good job

@rtorrero rtorrero force-pushed the navigate-filtered-checkresults branch from c7dfd3b to 577cc7d Compare October 10, 2022 10:07
@rtorrero rtorrero requested a review from arbulu89 October 10, 2022 10:26
arbulu89
arbulu89 approved these changes Oct 10, 2022
View reviewed changes
Copy link
Contributor

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

Thanks @rtorrero

@rtorrero rtorrero merged commit 1d37b84 into trento-project:main Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arbulu89 arbulu89 arbulu89 approved these changes

@dottorblaster dottorblaster dottorblaster approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request ux
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rtorrero @dottorblaster @arbulu89