-
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
Enable right click on visualizations and dashboards listings #88936
Enable right click on visualizations and dashboards listings #88936
Conversation
@elasticmachine merge upstream |
Pinging @elastic/kibana-app (Team:KibanaApp) |
Pinging @elastic/kibana-presentation (Team:Presentation) |
@elasticmachine merge upstream |
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.
Hey @stratoula ! Thanks for the enhancement, this is great!
This just need couple of issues to be resolved:
- Navigating to different apps from Visualize app (e.x. Maps or Lens) causing a full page reload:
Screen.Recording.2021-01-26.at.13.00.49.mov
Wrapping the link component with RedirectAppLinks
resolves the problem:
I'm not sure that Dashboard has the same problem since it doesn't have links to external apps, right?
- Other parameters from the global state are not persisted - filters, refresh interval, query.
That means all the state of pinned filters or refresh interval will be lost. Should be persisted in the url too.
src/plugins/visualize/public/application/utils/get_visualize_list_item_link.ts
Outdated
Show resolved
Hide resolved
@elasticmachine merge upstream |
Thanx @sulemanof ❤️ I had totally missed the global state params. I think that it should be ok now |
@elasticmachine merge upstream |
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.
Code LGTM 👍
Tested functionality locally with navigations to different apps, works as expected!
Thanks for switching to kbnUrlStateStorage
, it looks better FMPOV 🤔
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.
Tested locally in chrome, everything works great, including right click, and opening a dashboard with a saved time range in a new tab.
src/plugins/dashboard/public/application/listing/get_dashboard_list_item_link.ts
Outdated
Show resolved
Hide resolved
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
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.
LGTM. Tested on Chrome and Firefox
…#88936) * Enable right-click on visualizations listing page * Make it simpler * Enable right click to the dashboard listing * Add unit tests * Fix link on dashboard * Fix visualize link * Fix PR comments * Fix functional test failure * Use kbnUrlStateStorage instead * Change method to getDashboardListItemLink * Change method to getVisualizeListItemLink Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
…#89558) * Enable right-click on visualizations listing page * Make it simpler * Enable right click to the dashboard listing * Add unit tests * Fix link on dashboard * Fix visualize link * Fix PR comments * Fix functional test failure * Use kbnUrlStateStorage instead * Change method to getDashboardListItemLink * Change method to getVisualizeListItemLink Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Summary
Closes #88702. This PR enables the right click on the titles of the dashboards and visualizations listings.
EuiLink
has been replaced with href. The function checks if the dashboards have the timerange saved. If not, it propagates the timeRange of the queryService to the URL. A unit test has been added.EuiLink
has been replaced with href. The timeRange of the queryService is also propagated to the URL. A unit test has been added.Checklist
Delete any items that are not applicable to this PR.