-
Notifications
You must be signed in to change notification settings - Fork 923
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
[Dashboard] Fix bugs on removing pinned global filters on dashboard page #5143
[Dashboard] Fix bugs on removing pinned global filters on dashboard page #5143
Conversation
Signed-off-by: abbyhu2000 <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #5143 +/- ##
==========================================
- Coverage 66.74% 66.72% -0.03%
==========================================
Files 3278 3278
Lines 62985 62988 +3
Branches 10029 10029
==========================================
- Hits 42040 42027 -13
- Misses 18475 18490 +15
- Partials 2470 2471 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@@ -193,6 +193,17 @@ export const useDashboardAppAndGlobalState = ({ | |||
|
|||
subscriptions.add(stopSyncingFromTimeFilters); | |||
|
|||
const stopSyncingFromGlobalFilters = filterManager.getUpdates$().subscribe(() => { |
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.
Shouldnt we unsubscribe from this when exiting the hook?
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 did unsubscribe by subscriptions.add(stopSyncingFromGlobalFilters);
and subscriptions.unsubscribe();
@abbyhu2000 any plans to also add a regression test for this? Maybe in the functional tests? |
Signed-off-by: abbyhu2000 <[email protected]> Co-authored-by: Miki <[email protected]> Co-authored-by: Josh Romero <[email protected]> (cherry picked from commit ab925eb) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
(cherry picked from commit ab925eb) Signed-off-by: abbyhu2000 <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Miki <[email protected]> Co-authored-by: Josh Romero <[email protected]>
Signed-off-by: abbyhu2000 <[email protected]> Co-authored-by: Miki <[email protected]> Co-authored-by: Josh Romero <[email protected]> Signed-off-by: Leo Deng <[email protected]>
Signed-off-by: abbyhu2000 <[email protected]> Co-authored-by: Miki <[email protected]> Co-authored-by: Josh Romero <[email protected]> Signed-off-by: Leo Deng <[email protected]>
Signed-off-by: abbyhu2000 <[email protected]> Co-authored-by: Miki <[email protected]> Co-authored-by: Josh Romero <[email protected]> Signed-off-by: Willie Hung <[email protected]>
Signed-off-by: abbyhu2000 <[email protected]> Co-authored-by: Miki <[email protected]> Co-authored-by: Josh Romero <[email protected]>
Description
This PR fixes the error removing pinned global filters.
Previously removing pinned global filters needs a refresh for it to be effective on the dashboard because it does not get updated immediately from global states to the dashboard container. Previously the dashboard container only listens to the app filters, this PR adds a listener on the global filter for the dashboard container.
Issues Resolved
resolves #4767
Screenshot
Screen.Recording.2023-09-27.at.10.34.53.PM.mov