Add embed mode options in the Share UI by ajwild · Pull Request #58435 · elastic/kibana

ajwild · 2020-02-25T08:19:08Z

Summary

This addresses a few of the capabilities listed in #9575 - specifically showing the query input, date picker and filters. I haven't looked into the zooming and panel moving/resizing options yet.

I also made it possible to hide the top nav menu items (Full Screen, Share, Clone, Edit) but this exposed a bug so it may be simpler to remove this functionality if it's not really required.

I'll add a few comments below where some feedback would be appreciated.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

kibanamachine · 2020-02-25T08:19:10Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

timroes · 2020-02-25T08:21:35Z

Jenkins, test this

elasticmachine · 2020-02-25T08:22:13Z

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

src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app_controller.tsx

src/plugins/dashboard_embeddable_container/public/plugin.tsx

ajwild · 2020-02-25T08:38:15Z

src/plugins/data/public/ui/query_string_input/query_bar_top_row.tsx

      responsive={!!props.showDatePicker}
      gutterSize="s"
-      justifyContent="flexEnd"
+      justifyContent="flexStart"


I've made this change because the date picker looks out of place when it's right aligned and the query input is not displayed - mostly because the filter bar is left aligned below it. Is there any harm in doing this?

I don't see any harm in doing this.

It affects visualizations that hide the query bar (Timelion, TSVB). It's probably a matter of taste what's worse - moving the date picker from its regular place or having a right-aligned line in the layout @cchaos what do you think?

This is how TSVB looks with this change:

Master state

Personally I have a slight preference for flexEnd, but no strong opinion

I generally prefer the right alignment, and I feel keeping the date picker location consistent is a good thing. However, in embedded mode this may not be as important because the end user may not be a frequent Kibana user, and I think that it looks out of place when there is content on the lines above and below like this:

I can make this a prop so that it doesn't impact the other views, but it would be good to hear some more thoughts. I also have no strong opinion on this, but I made the change to find out if it's preferred. I'd be happy to change it back.

I agree with you @ajwild. We are aware that this is not ideal and are continuing a search for a better solution. The better solution though involves lots of folks across different teams coming to a conclusion so it's taking a while to nail down. Consistency is the main factor here, and we should try to keep the global time filters in the same place across all apps (even when no query bar exists).

In embed mode 🤷‍♀ , it's probably okay to shift.

@ajwild I like the prop solution in that case

I think it's okay to do a shift, as long as it's not complicating the API \ code.
As the design of this will soon change anyway, I think we could go with an 80:20 solution, allowing having the datepicker in embedded mode, even if it's not perfectly positioned ATM.

src/plugins/navigation/public/top_nav_menu/top_nav_menu.tsx

src/plugins/share/public/components/url_panel_content.tsx

src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app_controller.tsx

src/plugins/share/public/components/url_panel_content.tsx

majagrubic · 2020-02-25T12:23:54Z

src/plugins/share/public/components/url_panel_content.tsx

+      ['&show-top-nav-menu=true', this.state.showTopNavMenu],
+      ['&show-query-input=true', this.state.showQueryInput],
+      ['&show-date-picker=true', this.state.showDatePicker],
+      ['&hide-filter-bar=true', !this.state.showFilterBar], // Inverted to keep default behaviour for old links


Can we name all parameters hide-something for consistency?

I'd prefer show, just so we're consistent with our own flags.

If I change it to use &show-filter-bar=true, then all previously generated links (which users may have already embedded into websites etc.) will begin hiding the filter bar because this value won't be set.

Edit: This would also be the case if switching to &hide-* for the other parameters - it depends what default are wanted for older links. An alternative approach would be to implement =false, and then if the value is missing then the current defaults could be used.

Well, then lets go for hide-something like @majagrubic suggested?

src/plugins/navigation/public/top_nav_menu/top_nav_menu.tsx

majagrubic

Thanks for deciding to contribute to Kibana. I've tested this and it works well. Some overall comments:

This will introduce the new options in all the places where UrlPanelComponent is used, not just the dashboard - eg. Visualize, and since Visualize code hasn't been changed to make use of these parameters, the embeddable renders it without nav. I think we should add a dashboard-specific UrlPanelComponent and make this change for Dashboard only.
The "showFilter" switch doesn't seem to work properly - regardless if it's switched on/off the url parameter is hide-filter-bar=true.

flash1293

That's great @ajwild - thanks for looking into this! I added a few suggestions, but this is looking pretty good already.

src/plugins/dashboard_embeddable_container/public/plugin.tsx

flash1293 · 2020-02-26T09:44:04Z

src/plugins/data/public/ui/query_string_input/query_bar_top_row.tsx

      responsive={!!props.showDatePicker}
      gutterSize="s"
-      justifyContent="flexEnd"
+      justifyContent="flexStart"


It affects visualizations that hide the query bar (Timelion, TSVB). It's probably a matter of taste what's worse - moving the date picker from its regular place or having a right-aligned line in the layout @cchaos what do you think?

This is how TSVB looks with this change:

Master state

Personally I have a slight preference for flexEnd, but no strong opinion

src/plugins/share/public/components/url_panel_content.tsx

gchaps · 2020-02-27T20:59:58Z

How about adding a label over the "Show" toggle buttons to reduce the amount of text the user needs to read. Here is my suggested wording:

Include

Top menu
Query
Time filter
Filter bar

Also, for these four items, I don't feel a tooltip is needed.

cchaos · 2020-02-27T21:20:49Z

I agree with @gchaps. And at that point, the group should just be a EuiCheckboxGroup (for spacing and accessibility concerns) which you will be able to find an example of on this page: https://elastic.github.io/eui/#/forms/form-controls and use an EuiHorizontalRule to separate the concerns.

Let me know if you'd like help implementing these UI controls.

ajwild · 2020-02-27T23:04:48Z

Thanks for all the feedback! I hope to make the changes requested on Monday, and I'll reach out if any questions come up.

lizozom · 2020-03-11T13:31:44Z

@elasticmachine merge upstream

ajwild · 2020-06-07T14:33:32Z

@flash1293 it wasn't a misunderstanding, it was something I overlooked when refactoring (see #58435 (comment)). It should be working as expected now. I've rebased and fixed a merge conflict too.

flash1293 · 2020-06-08T08:00:59Z

@elasticmachine merge upstream

flash1293 · 2020-06-08T08:01:23Z

Jenkins, test this

kibanamachine · 2020-06-08T09:43:26Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 33e8fc4

History

💚 Build #51326 succeeded 7a397621c51b4c12ca6fb07d1ada864fbd21ccbd
💔 Build #51309 failed 0a2a504e93bc3519455801d8a53cc5d765506143
💔 Build #50556 failed 7fd2e18db751af8a52fe159d1e4ccc605814f398
💚 Build #49149 succeeded 9fb085033c333f670ba06b088a9690a05587d026
💔 Build #49123 failed 6e55e8189b0dc4acd913e4b2b619729747ecb42f

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

flash1293

Tested in Chrome and Firefox, LGTM. Thanks a lot @ajwild for this great contribution!

ajwild · 2020-06-08T12:14:37Z

No problem! Thanks to you and everyone else involved for all the helpful feedback and patience.

# Conflicts: # src/plugins/dashboard/public/application/embeddable/viewport/dashboard_viewport.tsx

* master: [ML] DFAnalytics results: ensure ml result fields are shown in data grid (elastic#68305) [security_solution] enable react-hooks/exhaustive-deps (elastic#68470) Closes elastic#66867 by adding missing, requried API params (elastic#68465) [Telemetry] collect number of visualization saved in the past 7, 30 and 90 days (elastic#67865) [Logs UI] View in context tweaks (elastic#67777) Kibana developer examples landing page (elastic#67049) Bump decompress package version (elastic#68386) fix elastic#66185 (elastic#66186) Bump pdfmake package version (elastic#68395) Unskip embeddables/adding_children suite (elastic#68111) Add embed mode options in the Share UI (elastic#58435) Adding key to avoid react warning (elastic#68491)

LeeDr · 2020-06-16T14:57:40Z

If you don't include the time filter, what time is used? Is the current time range from the URL included? Does the iFrame user know what the time range is? If it's a relative time range does that still work?

flash1293 · 2020-06-16T14:58:49Z

@LeeDr It's using the time range from the url (which can also be relative by using datemath involving now).

The time range is not shown anywhere to the iframe user, but that's just how it worked all the time.

RonnieDilli · 2020-06-19T14:27:37Z

Amazing news guys, these features are deal breakers for us!
Do you have any idea when this might be released?

flash1293 · 2020-06-19T14:54:32Z

@RonnieDilli it will be released in the upcoming version 7.9

fbaligand · 2020-06-22T13:05:52Z

Great new feature!
Really interested to use it in my professional projects!

periyasamy-sbna · 2020-06-24T09:31:42Z

Amazing future. I'm waiting for this release.

pgraca-38 · 2020-10-06T13:51:51Z

Hello, pushing that up. Any news on when this will be merged? thanks

flash1293 · 2020-10-06T14:04:22Z

@pgraca-38 it's already released with 7.9

pgraca-38 · 2020-10-06T14:08:06Z

Thank You! Completly missed it )

ajwild requested a review from a team February 25, 2020 08:19

ajwild requested review from a team as code owners February 25, 2020 08:19

timroes added Feature:SharingURLs Short URLs and Share URL features Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// labels Feb 25, 2020

timroes added the Feature:Dashboard Dashboard related features label Feb 25, 2020

timroes assigned flash1293 Feb 25, 2020

timroes requested a review from majagrubic February 25, 2020 08:28

ajwild commented Feb 25, 2020

View reviewed changes

src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app_controller.tsx Outdated Show resolved Hide resolved

ajwild commented Feb 25, 2020

View reviewed changes

src/plugins/dashboard_embeddable_container/public/plugin.tsx Outdated Show resolved Hide resolved

ajwild commented Feb 25, 2020

View reviewed changes

src/plugins/navigation/public/top_nav_menu/top_nav_menu.tsx Outdated Show resolved Hide resolved

ajwild commented Feb 25, 2020

View reviewed changes

src/plugins/share/public/components/url_panel_content.tsx Outdated Show resolved Hide resolved

timroes assigned majagrubic Feb 25, 2020

majagrubic reviewed Feb 25, 2020

View reviewed changes

src/legacy/core_plugins/kibana/public/dashboard/np_ready/dashboard_app_controller.tsx Outdated Show resolved Hide resolved

majagrubic reviewed Feb 25, 2020

View reviewed changes

src/plugins/share/public/components/url_panel_content.tsx Outdated Show resolved Hide resolved

majagrubic reviewed Feb 25, 2020

View reviewed changes

src/plugins/navigation/public/top_nav_menu/top_nav_menu.tsx Outdated Show resolved Hide resolved

majagrubic reviewed Feb 25, 2020

View reviewed changes

flash1293 reviewed Feb 26, 2020

View reviewed changes

ajwild force-pushed the embed-mode-options branch from bbca5a7 to 0632065 Compare March 4, 2020 12:41

lizozom added release_note:skip Skip the PR/issue when compiling release notes v7.7.0 labels Mar 11, 2020

ajwild added 3 commits June 7, 2020 14:05

Use helpers in functional test

9bb73dd

Pass strings to i18n.translate function

d1ff102

Fix ESLint errors

aac8b12

ajwild force-pushed the embed-mode-options branch from 7a39762 to 9c977f1 Compare June 7, 2020 14:15

ajwild requested a review from flash1293 June 7, 2020 14:16

Hide components in full screen when embedded

417abf0

ajwild force-pushed the embed-mode-options branch from 9c977f1 to 417abf0 Compare June 7, 2020 14:26

Merge branch 'master' into embed-mode-options

33e8fc4

flash1293 approved these changes Jun 8, 2020

View reviewed changes

flash1293 merged commit 38b9a8d into elastic:master Jun 8, 2020

flash1293 mentioned this pull request Jun 8, 2020

[7.x] Add embed mode options in the Share UI (#58435) #68503

Merged

flash1293 pushed a commit to flash1293/kibana that referenced this pull request Jun 8, 2020

Add embed mode options in the Share UI (elastic#58435)

8981630

# Conflicts: # src/plugins/dashboard/public/application/embeddable/viewport/dashboard_viewport.tsx

flash1293 added a commit that referenced this pull request Jun 8, 2020

Add embed mode options in the Share UI (#58435) (#68503)

823ce64

RonnieDilli mentioned this pull request Jun 19, 2020

Support for Kibana 7.6 nreese/kibana-time-plugin#64

Open

nreese mentioned this pull request Mar 10, 2022

Visualization editor doesn't provide any way to save when embedded #127308

Closed

Conversation

ajwild commented Feb 25, 2020

Summary

Checklist

For maintainers

Uh oh!

kibanamachine commented Feb 25, 2020

Uh oh!

timroes commented Feb 25, 2020

Uh oh!

elasticmachine commented Feb 25, 2020

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ajwild Mar 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

majagrubic left a comment

Choose a reason for hiding this comment

Uh oh!

flash1293 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gchaps commented Feb 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cchaos commented Feb 27, 2020

Uh oh!

ajwild commented Feb 27, 2020

Uh oh!

lizozom commented Mar 11, 2020

Uh oh!

ajwild commented Jun 7, 2020

Uh oh!

flash1293 commented Jun 8, 2020

Uh oh!

flash1293 commented Jun 8, 2020

Uh oh!

kibanamachine commented Jun 8, 2020

💚 Build Succeeded

History

Uh oh!

flash1293 left a comment

Choose a reason for hiding this comment

Uh oh!

ajwild commented Jun 8, 2020

Uh oh!

LeeDr commented Jun 16, 2020

Uh oh!

flash1293 commented Jun 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

ajwild Mar 4, 2020 •

edited

Loading

gchaps commented Feb 27, 2020 •

edited

Loading

flash1293 commented Jun 16, 2020 •

edited

Loading

fbaligand commented Jun 22, 2020 •

edited

Loading