[Presentation Util] Shared toolbar component by cqliu1 · Pull Request #94139 · elastic/kibana

cqliu1 · 2021-03-09T16:35:17Z

Summary

Related to #85666.

This moves the solution toolbar from the dashboard plugin into the presentation_util plugin and adds quick buttons to the toolbar in Dashboard.

Significant changes:

Moved panelToolbar into presentationUtil plugin and renamed to solutionToolbar
Quick buttons for Markdown and Input controls were added to Dashboard toolbar
@elastic/kibana-operations The presentationUtil plugin bundle size limit was increased because we added new toolbar components. The toolbar should be one of the first visible parts of the UI in Dashboard, so it shouldn't be lazy loaded.

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
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

Fixed ts errors Fixed tsconfig Fixed quick button background color Suggestions for more generic component; named slots projection Refactor toolbar in dashboard Fixed i18n ids Added button size to QuickButtonGroup Fixed add panel test subject Exported all sub components from solution toolbar Fixed library button data test subject Fixed style lint errors Spread remaining props on add from library button component Fixed prop type Added functional tests for dashboard quick buttons Use page object instead of test subject Fixed functional tests Renamed files Fixed toolbar components

elasticmachine · 2021-04-07T16:59:48Z

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

jbudz

operations - limits.yml LGTM

ThomThomson

The code looks super modular and well done!

Also tested in Chrome, and the shortcut buttons work really well and everything mostly seems to be working as expected.

One thing I did notice is that the unsaved changes badge appears if you navigate to an editor, then return to the dashboard with the back button or the cancel button. There shouldn't any unsaved changes in this state.
I'm not sure if this is in Master or not - but I checked and it works correctly in 7.12.

Edit turns out that I can reproduce this issue on master as well. I will investigate!
Edit 2 It turns out that this only happens when you have a map panel on a dashboard, I have created an issue

poffdeluxe

Looks great! One small question but otherwise looks good

poffdeluxe · 2021-04-07T23:00:33Z

src/plugins/presentation_util/public/components/solution_toolbar/items/primary_popover.tsx

+
+// required for dynamic import using React.lazy()
+// eslint-disable-next-line import/no-default-export
+export default PrimaryActionPopover;


A lot of these have the export default but are we using the React.lazy for these components? Do we still want to export them like this?

Ah right, that's leftover from when I was trying to lazy load the components, but then Clint helped me realize that we probably shouldn't lazy load the toolbar. I'll remove the default export from all the solution toolbar components.

MichaelMarcialis

Left one small SCSS comment below, but looks good otherwise. Approving so I don't hold you up.

MichaelMarcialis · 2021-04-08T17:03:06Z

src/plugins/presentation_util/public/components/solution_toolbar/items/quick_group.scss

@@ -0,0 +1,8 @@
+.quickButtonGroup {
+  .euiButtonGroup__buttons {


Would it be possible to create/use a different class to avoid using EUI classes? Thanks!

Actually, on second look, it appears that the related border-radius style might not even be necessary, as EUI is already giving it this styling. If I'm not mistaken, I think this selector and style can be removed altogether.

stratoula · 2021-04-12T14:04:39Z

@elasticmachine merge upstream

kibanamachine · 2021-04-12T14:04:40Z

user doesn't have permission to update head repository

stratoula · 2021-04-12T14:17:59Z

src/plugins/visualizations/public/wizard/new_vis_modal.tsx

      return null;
    }

+    if (this.props.newVisType) {


I am so sorry but I dont get what we want to do. What kind of information has the newVisType prop. And why do we return null if it has a value? If we don't want to display the wizard, why do we use this component? 🤔

So it turns out I didn't actually need to go through the new vis modal to add these visualizations, and instead use the embeddable state transfer service. I removed all the changes to the new vis modal.

stratoula

LGTM, thank you @cqliu1 for addressing all my feedback!

stratoula · 2021-04-13T12:57:47Z

src/plugins/visualizations/public/embeddable/visualize_embeddable.ts

  query?: Query;
  filters?: Filter[];
  timeRange?: TimeRange;
+  newVisType?: string;


You can also remove this, right?

Yep! Sorry I missed this line.

kibanamachine · 2021-04-13T16:22:58Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`dashboard`	200	189	-11
`presentationUtil`	50	87	+37
total			+26

Async chunks

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

id	before	after	diff
`dashboard`	134.8KB	132.6KB	-2.2KB
`presentationUtil`	46.5KB	53.3KB	+6.8KB
total			+4.6KB

Page load bundle

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

id	before	after	diff
`presentationUtil`	15.2KB	33.7KB	+18.5KB
`visualizations`	92.5KB	92.5KB	-20.0B
total			+18.5KB

History

💚 Build #118934 succeeded c6b746d
💔 Build #118855 failed 5437689
💔 Build #118857 failed bd87a73
💚 Build #118439 succeeded 52eb3aa
💔 Build #118401 failed 2afea6f

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

cqliu1 force-pushed the pres-util/panel-toolbar branch 2 times, most recently from ce80f02 to 51ae1ae Compare March 11, 2021 20:52

cqliu1 mentioned this pull request Mar 11, 2021

[Canvas] Integrates Solution Toolbar #94489

Closed

9 tasks

cqliu1 force-pushed the pres-util/panel-toolbar branch 2 times, most recently from 45c848b to 847f278 Compare March 25, 2021 17:53

cqliu1 force-pushed the pres-util/panel-toolbar branch 5 times, most recently from c0c85a7 to c80a919 Compare April 2, 2021 21:01

cqliu1 added Feature:Dashboard Dashboard related features Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// labels Apr 2, 2021

cqliu1 mentioned this pull request Apr 5, 2021

[Dashboard] Makes lens default editor for creating new panels #96181

Merged

7 tasks

cqliu1 added enhancement New value added to drive a business result release_note:enhancement v7.13.0 v8.0.0 loe:x-large Extra Large Level of Effort impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. labels Apr 6, 2021

cqliu1 force-pushed the pres-util/panel-toolbar branch 4 times, most recently from 4e8fd95 to dc91fa6 Compare April 7, 2021 01:49

cqliu1 force-pushed the pres-util/panel-toolbar branch from dc91fa6 to 68613cf Compare April 7, 2021 16:52

cqliu1 marked this pull request as ready for review April 7, 2021 16:59

cqliu1 requested a review from a team April 7, 2021 16:59

cqliu1 requested review from a team as code owners April 7, 2021 16:59

jbudz approved these changes Apr 7, 2021

View reviewed changes

Removed panel toolbar component

fdcde37

ThomThomson reviewed Apr 7, 2021

View reviewed changes

ThomThomson approved these changes Apr 7, 2021

View reviewed changes

poffdeluxe approved these changes Apr 7, 2021

View reviewed changes

cqliu1 added 2 commits April 7, 2021 19:44

Renamed folder

c0713b3

Removed unnecessary default exports

9c83153

MichaelMarcialis approved these changes Apr 8, 2021

View reviewed changes

cqliu1 added 2 commits April 8, 2021 12:50

Removed border radius from quick button

2afea6f

Merge branch 'master' into pres-util/panel-toolbar

52eb3aa

elastic deleted a comment from kibanamachine Apr 12, 2021

stratoula reviewed Apr 12, 2021

View reviewed changes

cqliu1 added 6 commits April 12, 2021 10:03

Use embeddable state transfer navigate to editor

848a6d6

Removed newVisType prop from new vis modal

7f031a6

Use dashboard constant

5437689

Removed comments

bd87a73

Fixed embeddable factory

8206bed

Merge branch 'master' into pres-util/panel-toolbar

c6b746d

stratoula approved these changes Apr 13, 2021

View reviewed changes

stratoula reviewed Apr 13, 2021

View reviewed changes

Removed remaining newVisType property

f8326aa

cqliu1 merged commit 74d93a2 into elastic:master Apr 13, 2021

cqliu1 deleted the pres-util/panel-toolbar branch April 13, 2021 16:24

cqliu1 mentioned this pull request Apr 13, 2021

[7.x] [Presentation Util] Shared toolbar component (#94139) #97003

Merged

cqliu1 added a commit that referenced this pull request Apr 13, 2021

[Presentation Util] Shared toolbar component (#94139) (#97003)

8365901

cqliu1 mentioned this pull request Sep 28, 2021

[Canvas] Toolbar UI Updates #113329

Merged

8 tasks

cqliu1 restored the pres-util/panel-toolbar branch June 8, 2022 13:57

		@@ -0,0 +1,8 @@
		.quickButtonGroup {
		.euiButtonGroup__buttons {

Conversation

cqliu1 commented Mar 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

elasticmachine commented Apr 7, 2021

Uh oh!

jbudz left a comment

Choose a reason for hiding this comment

Uh oh!

ThomThomson left a comment • 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!

poffdeluxe left a comment

Choose a reason for hiding this comment

Uh oh!

poffdeluxe Apr 7, 2021

Choose a reason for hiding this comment

Uh oh!

cqliu1 Apr 8, 2021

Choose a reason for hiding this comment

Uh oh!

MichaelMarcialis left a comment

Choose a reason for hiding this comment

Uh oh!

MichaelMarcialis Apr 8, 2021

Choose a reason for hiding this comment

Uh oh!

MichaelMarcialis Apr 8, 2021

Choose a reason for hiding this comment

Uh oh!

stratoula commented Apr 12, 2021

Uh oh!

kibanamachine commented Apr 12, 2021

Uh oh!

stratoula Apr 12, 2021 • 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!

cqliu1 Apr 12, 2021

Choose a reason for hiding this comment

Uh oh!

stratoula left a comment

Choose a reason for hiding this comment

Uh oh!

stratoula Apr 13, 2021

Choose a reason for hiding this comment

Uh oh!

cqliu1 Apr 13, 2021

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Apr 13, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

cqliu1 commented Mar 9, 2021 •

edited

Loading

ThomThomson left a comment •

edited

Loading

stratoula Apr 12, 2021 •

edited

Loading