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

[Dashboard First] Library Notification #76122

Merged

Conversation

ThomThomson
Copy link
Contributor

Summary

Closes #75499

Reasoning

Adds a notification which denotes when an embeddable is saved to the Visualize Library. Once the 'by value' paradigm is an accepted default, it will be important to be able to distinguish between 'by value' and 'by reference' embeddables at a glance, due to the fact that editing by reference embeddables can apply your changes to multiple dashboards.

This notification shows up when:

  • The Embeddable implements ReferenceOrValueEmbeddable i.e. is capable of taking input that contains either a savedObjectId OR the saved object attributes.
  • Is currently using 'by reference' type input.
  • Is on a container in edit mode.

Implementation

The Library notification is an EUI Badge using the hollow color scheme to make it stand out less. This badge could start showing up in a lot of places once the 'allowByValueEmbeddables' flag is turned on, so it's important to make it less conspicuous.

The custom badge is implemented via the MenuItem prop on action, which I have changed the notifications area of embeddable panel header to accept.

Screenshots

The Library notification on Book embeddables...
Screen Shot 2020-08-27 at 10 51 24 AM

The Library notification appearing on a Lens By Value #70272 embeddable. The Drilldown notification and local time range are shown for context.

Screen Shot 2020-08-19 at 4 09 24 PM

How to test this?

This is just one part of a much larger change, so all changes in this PR are hidden behind a configuration value.

  1. you'll need to set the allowByValueEmbeddables config value to true:

    allowByValueEmbeddables: schema.boolean({ defaultValue: false }),

  2. There are no production ready embeddables that implement the ReferenceOrValueEmbeddable yet so you will have to start the developer examples. yarn start --run-examples

  3. Open a dashboard, and use the add panels functionality to add a Book embeddable. You should see the new notification show up on the right side.

  4. Use the 'Unlink from Library' functionality to unlink the panel, and see that the notification disappears.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@ThomThomson ThomThomson added Feature:Dashboard Dashboard related features Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.10.0 labels Aug 27, 2020
@ThomThomson ThomThomson requested a review from a team August 27, 2020 17:21
@ThomThomson ThomThomson requested a review from a team as a code owner August 27, 2020 17:21
@elasticmachine
Copy link
Contributor

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

@botelastic botelastic bot added the Feature:Embedding Embedding content via iFrame label Aug 27, 2020
Copy link
Contributor

@streamich streamich 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.

@ThomThomson
Copy link
Contributor Author

@elasticmachine merge upstream

};

public isCompatible = async ({ embeddable }: LibraryNotificationActionContext) => {
return Boolean(
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think we can omit Boolean() here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Removed that

Copy link
Contributor

@stratoula stratoula 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 it on Chrome, works as expected

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
dashboard 177 +1 176

page load bundle size

id value diff baseline
dashboard 708.2KB +6.0KB 702.2KB
embeddable 430.7KB +137.0B 430.5KB
total +6.1KB

History

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

@ThomThomson ThomThomson merged commit 1ae2a14 into elastic:master Sep 1, 2020
ThomThomson added a commit to ThomThomson/kibana that referenced this pull request Sep 1, 2020
Added a notification to show when an embeddable is saved to the Visualize Library
ThomThomson added a commit that referenced this pull request Sep 1, 2020
Added a notification to show when an embeddable is saved to the Visualize Library
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 2, 2020
* master: (223 commits)
  skip flaky suite (elastic#75724)
  [Reporting] Add functional test for Reports in non-default spaces (elastic#76053)
  [Enterprise Search] Fix various icons in dark mode (elastic#76430)
  skip flaky suite (elastic#76245)
  Add `auto` interval to histogram AggConfig (elastic#76001)
  [Resolver] generator uses setup_node_env (elastic#76422)
  [Ingest Manager] Support both zip & tar archives from Registry (elastic#76197)
  [Ingest Manager] Improve agent vs kibana version checks (elastic#76238)
  Manually building `KueryNode` for Fleet's routes (elastic#75693)
  remove dupe tinymath section (elastic#76093)
  Create APM issue template (elastic#76362)
  Delete unused file. (elastic#76386)
  [SECURITY_SOLUTION][ENDPOINT] Trusted Apps Create API (elastic#76178)
  [Detections Engine] Add Alert actions to the Timeline (elastic#73228)
  [Dashboard First] Library Notification (elastic#76122)
  [Maps] Add mvt support for ES doc sources  (elastic#75698)
  Add setHeaderActionMenu API to AppMountParameters (elastic#75422)
  [ML] Remove "Are you sure" from data frame analytics jobs (elastic#76214)
  [yarn] remove typings-tester, use @ts-expect-error (elastic#76341)
  [Reporting/CSV] Do not fail the job if scroll ID can not be cleared (elastic#76014)
  ...
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:Embedding Embedding content via iFrame release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Discuss] Custom Badge to Denote Panel By Reference
5 participants