-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat(app): history navigation preserves filters #3404
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
base: master
Are you sure you want to change the base?
Conversation
previously, when navigating the history using `[`, `]` and `-`, filters were not saved, so if you went to pods, filtered for `/nginx`, then went to services and then back to pods, the `/nginx` filter would not be used and the default pod view would be shown. now, that filter is saved. closes derailed#3220
Tested this and it seems to work well. However, I noticed an issue that occurs specifically when filtering by label. Steps to reproduce: Go to Apply a label filter: Navigate to a different view Return to At this point, the correct filter is still applied, but it’s not displayed in the UI, which might be misleading. |
@uozalp this is technically a pre-existing bug as I can reproduce it on |
…ided curently, if you use `:svc -l app.kubernetes.io/instance=cert-manager` in k9s and then navigate history, when returning to the page the results are correct but the filter is not shown. here, we ensure that the table title string is not an empty string. reported in derailed#3404, but unrelated bug originally introduced in derailed#3293, which I have not reviewed for any other rippling effects.
…ided currently, if you use `:svc -l app.kubernetes.io/instance=cert-manager` in k9s and then navigate history, when returning to the page the results are correct but the filter is not shown. here, we ensure that the table title string is not an empty string. reported in derailed#3404, but unrelated bug originally introduced in derailed#3293, which I have not reviewed for any other rippling effects.
The filter title issue should be fixed by #3405, and does not need to be depended upon for merging or wait for this PR to be merged. |
…ided currently, if you use `:svc -l app.kubernetes.io/instance=cert-manager` in k9s and then navigate history, when returning to the page the results are correct but the filter is not shown. here, we ensure that the table title string is not an empty string. reported in derailed#3404, but unrelated bug originally introduced in derailed#3293, which I have not reviewed for any other rippling effects.
Oh, one side-effect of this is that the filters get saved to the config and upon restart, k9s restores them. I think I like this behavior, but perhaps it should be adapted so users do not start k9s and see filtered results. |
a06d43a
to
85932b2
Compare
@tyzbit Thank you for this update! Looks like we have conflicts, can you take a peek? |
It looks like doing a simple rebase results in broken code because this area of the code was refactored in master. |
previously, when navigating the history using
[
,]
and-
, filters were not saved, so if you went to pods, filtered for/nginx
, then went to services and then back to pods, the/nginx
filter would not be used and the default pod view would be shown. now, that filter is saved.closes #3220