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

fix: empty screen reset filter by default #5307

Merged
merged 2 commits into from Jan 15, 2024
Merged

fix: empty screen reset filter by default #5307

merged 2 commits into from Jan 15, 2024

Conversation

deboer-tim
Copy link
Collaborator

What does this PR do?

Fixes the regression caused by #4600. Prior to this the FilteredEmptyScreen would reset all filters to ''. The container list needed special processing to not remove the filters like 'is:running' so it was changed to an event, but none of the other lists respond to this event so none of the filters get cleared.

This sets a default behaviour of clearing the list so that none of the other lists need to respond to the event if they are fine just clearing the filter. If you do your own special processing, you just call e.preventDefault(). The search term is included as an event property because we need to set cancelable in the third argument.

The pods list has the tabs/is:running so I added the event handling there as well. Without this change clearing the filter would always return you to the first tab.

Screenshot / video of UI

N/A

What issues does this PR fix or reference?

Fixes #5288.

How to test this PR?

Enter random text in container, pods, and volumes/images view, and confirm that clearing the filter works (and leaves the is:running/stopped for containers/pods list).

@deboer-tim deboer-tim requested review from benoitf and a team as code owners December 18, 2023 17:25
@deboer-tim deboer-tim requested review from dgolovin and cdrage and removed request for a team December 18, 2023 17:25
Copy link
Collaborator

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

bugfix would require a non regression test

@slemeur
Copy link
Collaborator

slemeur commented Dec 22, 2023

👍

Fixes the regression caused by #4600. Prior to this the FilteredEmptyScreen
would reset all filters to ''. The container list needed special processing
to not remove the filters like 'is:running' so it was changed to an event,
but none of the other lists respond to this event so none of the filters get
cleared.

This sets a default behaviour of clearing the list so that none of the other
lists need to respond to the event if they are fine just clearing the filter.
If you do your own special processing, you just call e.preventDefault(). The
search term is included as an event property because we need to set
cancelable in the third argument.

The pods list has the tabs/is:running so I added the event handling there as
well. Without this change clearing the filter would always return you to the
first tab.

Fixes #5288.

Signed-off-by: Tim deBoer <git@tdeboer.ca>
@deboer-tim
Copy link
Collaborator Author

Rebased to latest and added regression and non-regression tests.

lint and format don't agree, use disable to allow double quotes.

Signed-off-by: Tim deBoer <git@tdeboer.ca>
Copy link
Contributor

@axel7083 axel7083 left a comment

Choose a reason for hiding this comment

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

LGTM

@deboer-tim deboer-tim merged commit 14f8b5f into main Jan 15, 2024
8 checks passed
@deboer-tim deboer-tim deleted the empty-filter branch January 15, 2024 15:35
@podman-desktop-bot podman-desktop-bot added this to the 1.7.0 milestone Jan 15, 2024
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.

Clear filter button is useless in Images page
5 participants