-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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] Sharing via link to an expanded panel #190086
Conversation
7b95e59
to
6d28f6e
Compare
Pinging @elastic/kibana-presentation (Team:Presentation) |
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.
Testing this, and it doesn't actually seem to be accomplishing the goal? After generating a link with a maximized panel ID, that link does include maximizedPanelId
in the URL, but it doesn't actually maximize the panel when I use that link:
Screen.Recording.2024-08-15.at.3.07.58.PM.mov
It's also triggering unsaved changes, which I don't think it should 🤔
@Heenawter I think the proposed flow should be that the new shareable url should go to a new tab vs just getting pasted within the same tab. The goal would be that the URL would be given to a completely different user who would then open it (I think this behavior is better mimicked by opening it in a new tab). I think this functionality works - including a giphy below of what I think the expected behavior should be (and it's not going to the listing page), let me know what you think?: Aug-20-2024.09-12-29.mp4 |
@rshen91 Hmm... Not sure I agree :) I think the fact that this only works from a fresh tab is a hint that there is an underlying implementation issue. If I have Kibana open on two tabs and paste the generated URL from one into the other, it also doesn't work, even if I am on a completely unrelated dashboard - I don't believe it is fair to assume that the link will only ever be used on a new tab. It should just... work... regardless of where the user "started" and/or where the link is being pasted from. What do you think? |
src/plugins/dashboard/public/dashboard_app/dashboard_app.test.tsx
Outdated
Show resolved
Hide resolved
src/plugins/dashboard/public/dashboard_app/dashboard_app.test.tsx
Outdated
Show resolved
Hide resolved
src/plugins/dashboard/public/dashboard_app/dashboard_app.test.tsx
Outdated
Show resolved
Hide resolved
src/plugins/dashboard/public/dashboard_app/dashboard_app.test.tsx
Outdated
Show resolved
Hide resolved
src/plugins/dashboard/public/dashboard_app/dashboard_app.test.tsx
Outdated
Show resolved
Hide resolved
src/plugins/dashboard/public/dashboard_app/dashboard_app.test.tsx
Outdated
Show resolved
Hide resolved
src/plugins/dashboard/public/dashboard_app/dashboard_app.test.tsx
Outdated
Show resolved
Hide resolved
src/plugins/dashboard/public/dashboard_app/dashboard_app.test.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Hannah Mudge <[email protected]>
src/plugins/dashboard/public/dashboard_app/dashboard_app.test.tsx
Outdated
Show resolved
Hide resolved
src/plugins/dashboard/public/dashboard_app/dashboard_app.test.tsx
Outdated
Show resolved
Hide resolved
src/plugins/dashboard/public/dashboard_app/dashboard_app.test.tsx
Outdated
Show resolved
Hide resolved
src/plugins/dashboard/public/dashboard_app/dashboard_app.test.tsx
Outdated
Show resolved
Hide resolved
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.
Yay!! The test looks so much better now! And it is now way clearer what it is trying to do.
I think the takeaway here is to just really try to focus in on specifically what you are trying to test when writing unit tests. It really requires defining a "scope" and sticking to it - you can assume anything outside that scope is behaving as expected.
Tested locally + code review. Everything works as expected!! 🎉 🎉 🎉
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @rshen91 |
## Summary Closes elastic#145454 This PR allows users with a dashboard with an expanded (maximized) panel to be shared to other users via url or the share modal link. To implement this, the expanded panel Id is added to the url: `/app/dashboard/{dashboardID}/{expandedPanelID}_g()_a()` ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios Co-authored-by: Hannah Mudge <[email protected]> (cherry picked from commit 6030231)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…93458) # Backport This will backport the following commits from `main` to `8.x`: - [[Dashboard] Sharing via link to an expanded panel (#190086)](#190086) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Rachel Shen","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-19T17:38:21Z","message":"[Dashboard] Sharing via link to an expanded panel (#190086)\n\n## Summary\r\n\r\nCloses https://github.com/elastic/kibana/issues/145454\r\n\r\nThis PR allows users with a dashboard with an expanded (maximized) panel\r\nto be shared to other users via url or the share modal link. To\r\nimplement this, the expanded panel Id is added to the url:\r\n\r\n`/app/dashboard/{dashboardID}/{expandedPanelID}_g()_a()`\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\nCo-authored-by: Hannah Mudge <[email protected]>","sha":"603023124681429ee900ff3a73793ce31a9cad58","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Presentation","loe:medium","impact:medium","v9.0.0","backport:prev-minor","v8.16.0"],"title":"[Dashboard] Sharing via link to an expanded panel","number":190086,"url":"https://github.com/elastic/kibana/pull/190086","mergeCommit":{"message":"[Dashboard] Sharing via link to an expanded panel (#190086)\n\n## Summary\r\n\r\nCloses https://github.com/elastic/kibana/issues/145454\r\n\r\nThis PR allows users with a dashboard with an expanded (maximized) panel\r\nto be shared to other users via url or the share modal link. To\r\nimplement this, the expanded panel Id is added to the url:\r\n\r\n`/app/dashboard/{dashboardID}/{expandedPanelID}_g()_a()`\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\nCo-authored-by: Hannah Mudge <[email protected]>","sha":"603023124681429ee900ff3a73793ce31a9cad58"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/190086","number":190086,"mergeCommit":{"message":"[Dashboard] Sharing via link to an expanded panel (#190086)\n\n## Summary\r\n\r\nCloses https://github.com/elastic/kibana/issues/145454\r\n\r\nThis PR allows users with a dashboard with an expanded (maximized) panel\r\nto be shared to other users via url or the share modal link. To\r\nimplement this, the expanded panel Id is added to the url:\r\n\r\n`/app/dashboard/{dashboardID}/{expandedPanelID}_g()_a()`\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\nCo-authored-by: Hannah Mudge <[email protected]>","sha":"603023124681429ee900ff3a73793ce31a9cad58"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Rachel Shen <[email protected]>
Summary
Closes #145454
This PR allows users with a dashboard with an expanded (maximized) panel to be shared to other users via url or the share modal link. To implement this, the expanded panel Id is added to the url:
/app/dashboard/{dashboardID}/{expandedPanelID}_g()_a()
Checklist