-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
feat(ui): Improve UX when filtering applications (#4403) #4622
Conversation
4c6f801
to
ffad846
Compare
- When application filters are active, show a badge with # filters and a button to clear them - When no matching applications are found, provide a button to clear filters - Styling updates to application filters UI Signed-off-by: Tim Etchells <[email protected]>
ffad846
to
3cddf22
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.
Awesome changes @tetchel . Found one issue ( details in comment ). Let me know please if you need help fixing it.
<i | ||
className='fa fa-times' | ||
onClick={() => { | ||
AppsListPreferences.clearFilters(this.props.pref); |
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.
I've reproduced a bug.
- Navigate to the applications list
- Add project filter
- Click "Clear filters"
Projects tags input control keep showing the selected project. I think this is an old bug in https://github.com/argoproj/argo-cd/blob/master/ui/src/app/shared/components/tags-input/tags-input.tsx
getDerivedStateFromProps()
handler is missing and component does not support changing input
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.
I believe I've fixed it - I should have noticed that one when testing.
Signed-off-by: Tim Etchells <[email protected]>
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.
LGTM. Thank you!
Signed-off-by: Tim Etchells [email protected]
Checklist:
Fixes #4403