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

[Dashboard Usability] Add unsaved filters to app state to url #197313

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from
Draft

[Dashboard Usability] Add unsaved filters to app state to url #197313

wants to merge 21 commits into from
+135 −41

Conversation

rshen91
Copy link
Contributor

@rshen91 rshen91 commented Oct 22, 2024 •
edited
Loading

Summary

Closes #188914

This PR allows users to open dashboards in a new tab from the dashboard listing page with unsaved and unpinned filters. Previously, filters needed to be pinned or the dashboard needed to be saved for filters to persist across tabs.

  • mas tests

Checklist

@rshen91 rshen91 mentioned this pull request Oct 22, 2024
Closed
11 tasks
@rshen91 rshen91 self-assigned this Oct 22, 2024
@rshen91 rshen91 added release_note:enhancement backport:prev-major Backport to (8.x, 8.16, 8.15) the previous major branch and all later branches still in development backport:all-open Backport to all branches that could still receive a release Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas labels Oct 23, 2024
@rshen91 rshen91 marked this pull request as ready for review October 23, 2024 22:59
@rshen91 rshen91 requested a review from a team as a code owner October 23, 2024 22:59
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@nreese nreese self-requested a review October 24, 2024 13:58
@nreese
Copy link
Contributor

nreese commented Oct 24, 2024

It looks like filters were not transferred when opening a map in a new tap from "Recently viewed" left nav. Is this supposed to be resolve in this PR since its part of the issue?

nreese
nreese reviewed Oct 24, 2024
View reviewed changes
src/plugins/dashboard/public/dashboard_app/listing_page/get_dashboard_list_item_link.ts Outdated
sessionStorage.getItem(DASHBOARD_STATE_SESSION_KEY) ?? '[]'
);

const unsavedFilters = spaceId && unsavedFiltersToUrl ? unsavedFiltersToUrl[spaceId] : undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should unsaved queries also be forwarded to be consistent?

Copy link
Contributor

@Heenawter Heenawter Oct 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

They should be, yes :) I honestly wonder if... all unsaved changes should be forwarded? I can't think of a good argument for why opening a dashboard in a new tab would have different behaviour than the onClick behaviour....

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can re-use the code that generates the share link so they are the same

@rshen91 rshen91 marked this pull request as draft October 24, 2024 16:01
@elasticmachine
Copy link
Contributor

elasticmachine commented Oct 24, 2024 •
edited
Loading

🤖 Jobs for this PR can be triggered through checkboxes. 🚧

ℹ️ To trigger the CI, please tick the checkbox below 👇

  • Click to trigger kibana-pull-request for this PR!
  • Click to trigger kibana-deploy-project-from-pr for this PR!

@elasticmachine
Copy link
Contributor

elasticmachine commented Oct 30, 2024 •
edited
Loading

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #28 / maps app embeddable save and return work flow edit existing map "before each" hook for "should lose its connection to the dashboard when creating new map"
  • [job] [logs] FTR Configs #28 / maps app embeddable save and return work flow edit existing map "before each" hook for "should lose its connection to the dashboard when creating new map"
  • [job] [logs] FTR Configs #26 / Options list control Interactions between options list and dashboard "after all" hook in "Interactions between options list and dashboard"
  • [job] [logs] FTR Configs #26 / Options list control Interactions between options list and dashboard "after all" hook in "Interactions between options list and dashboard"

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
dashboard 646.1KB 647.5KB +1.4KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
dashboard 51.8KB 51.6KB -199.0B

History

cc @rshen91

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nreese nreese nreese left review comments

@Heenawter Heenawter Heenawter left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@rshen91 rshen91

Labels
backport:all-open Backport to all branches that could still receive a release backport:prev-major Backport to (8.x, 8.16, 8.15) the previous major branch and all later branches still in development release_note:enhancement Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Dashboard usability] Filters are lost when users open dashboards in a new tab
4 participants
@rshen91 @elasticmachine @nreese @Heenawter