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.

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

Merged
merged 16 commits into from
Jan 28, 2021

Conversation

stratoula
Copy link
Contributor

@stratoula stratoula commented Jan 21, 2021

Summary

Closes #88702. This PR enables the right click on the titles of the dashboards and visualizations listings.

  • Dashboards: The onClick on the 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.

image

  • Visualizations: The onClick on the EuiLink has been replaced with href. The timeRange of the queryService is also propagated to the URL. A unit test has been added.

image

Checklist

Delete any items that are not applicable to this PR.

@stratoula stratoula changed the title Listings enable right click Enable right click on visualizations and dashboards listings Jan 21, 2021
@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@stratoula stratoula added v8.0.0 v7.12.0 release_note:enhancement Feature:Dashboard Dashboard related features Feature:Visualizations Generic visualization features (in case no more specific feature label is available) Team:Visualizations Visualization editors, elastic-charts and infrastructure Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas labels Jan 21, 2021
@stratoula stratoula marked this pull request as ready for review January 21, 2021 17:18
@stratoula stratoula requested a review from a team January 21, 2021 17:18
@stratoula stratoula requested a review from a team as a code owner January 21, 2021 17:18
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@elasticmachine
Copy link
Contributor

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

@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@stratoula stratoula requested a review from sulemanof January 25, 2021 08:27
Copy link
Contributor

@sulemanof sulemanof left a 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:

  1. 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 RedirectAppLinksresolves the problem:

image

I'm not sure that Dashboard has the same problem since it doesn't have links to external apps, right?

  1. 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.

@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@stratoula
Copy link
Contributor Author

Thanx @sulemanof ❤️ I had totally missed the global state params. I think that it should be ok now

@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@sulemanof sulemanof left a 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 🤔

Copy link
Contributor

@ThomThomson ThomThomson left a 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.

@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
dashboard 199 200 +1
visualize 80 81 +1
total +2

Async chunks

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

id before after diff
dashboard 158.6KB 159.3KB +710.0B
visualize 97.5KB 98.1KB +662.0B
total +1.3KB

Page load bundle

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

id before after diff
dashboard 295.5KB 295.6KB +93.0B
visualize 33.3KB 33.6KB +358.0B
total +451.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@dej611 dej611 left a 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

@stratoula stratoula merged commit b4931e6 into elastic:master Jan 28, 2021
stratoula added a commit to stratoula/kibana that referenced this pull request Jan 28, 2021
…#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>
stratoula added a commit that referenced this pull request Jan 28, 2021
…#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Dashboard Dashboard related features Feature:Visualizations Generic visualization features (in case no more specific feature label is available) release_note:enhancement Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make listing pages (Visualizations, Dashboards) to actually use links
6 participants