[Controls] Control Group Embeddable and Management Experience #111065

ThomThomson · 2021-09-02T22:45:31Z

Closes #101191
Closes #108296
Closes #112838
Closes #108292
Closes #112841

Summary

Replacement for #109138.

This PR introduces a Control Group embeddable to the input controls storybook. This embeddable is a container similar to the Dashboard Container, and has the ability to:

Add Controls - complete with an inline embeddable creating workflow.
Edit Controls - using the same workflow as above.
Rearrange Controls - using DndKit to allow for smooth dragging over multiple lines, keyboard controls, and dragging with custom widths.
Set widths of all Controls
Reactively switch the design of controls from One Line to Two Line

Gifs covering important features incoming once @andreadelrio is finished making it look nicer!

Todo next:

Use Redux Toolkit to more easily manage state for the Controls Group [Dashboard Controls] Create State Storage for Control Group #101194
Change Control Group to support multiple types of Control in the registry [Controls] Build Registry for Control Types #112838
Call to Action banner for when the dashboard is in Edit Mode and no controls are present.
Better validation and error reporting for Inline Editors
Replace Options List editor's Field & Index Pattern selector with the ones from Lens

…itDragDrop

…ing and removing controls

…g stub

…itDragDrop

mdefazio · 2021-09-22T14:59:01Z

I think some of the PR description ghosted on you.

poffdeluxe

Overall looks pretty good! Just a few nits

...ugins/presentation_util/public/components/controls/control_frame/control_frame_component.tsx

src/plugins/presentation_util/public/components/controls/control_frame/control_frame_strings.ts

...plugins/presentation_util/public/components/controls/control_group/editor/manage_control.tsx

...tion_util/public/components/controls/control_group/editor/manage_control_group_component.tsx

poffdeluxe · 2021-09-29T20:11:57Z

...ation_util/public/components/controls/control_types/options_list/options_list_embeddable.tsx

@@ -58,22 +61,19 @@ export const OPTIONS_LIST_CONTROL = 'optionsListControl';
 export interface OptionsListEmbeddableInput extends InputControlInput {
  field: string;
  indexPattern: string;
-  multiSelect: boolean;
+  singleSelect?: boolean;


just curious: what's the reason for this flip?

This is flipped because multiselect defaults to true. Making an optional singleSelect entry in the input is the easiest way to handle that because undefined is falsy. If you want multiselect you can just pass nothing!

It's the perfect time right for renaming input keys, because once this is released we'd have to write migrations for changes like this.

ryankeairns

A couple of items to consider in addition to the comments in the code:

Mismatched border-radius

Consider sorting selected items to the top

If this list is very long, it would be a pain to find all the currently selected values. Alternatively, a way to unselect all would be nice (this sounds like an EUI request).

...plugins/presentation_util/public/components/controls/control_group/editor/manage_control.tsx

ryankeairns · 2021-09-29T20:10:38Z

src/plugins/presentation_util/public/components/controls/control_group/control_group.scss

+$smallControl: $euiSize * 14;
+$mediumControl: $euiSize * 25;
+$largeControl: $euiSize * 50;
+$controlMinWidth: $euiSize * 14;


Does it make sense to just set $controlMinWidth: $smallControl since they are the same value?

src/plugins/presentation_util/public/components/controls/control_group/control_group.scss

ryankeairns · 2021-09-29T20:20:41Z

Also, FWIW, the dragging feels more natural than it did last time I tried this out. The gray bordered box/placeholder also reminds me of the size of the thing I'm about to drop which seems like a decent tradeoff (and better than the empty space from before).

src/plugins/presentation_util/public/components/controls/control_group/control_group.scss

andreadelrio · 2021-09-29T21:25:48Z

Mismatched border-radius

This is happening because we're doing $euiBorderRadius which in K8 is 6px and for some reason it seems like Storybook is using the K7 value for this variable which is 4px. We decided to leave it in thinking that once this makes its way to real Kibana it will get fixed (i.e. the right value for K8 will get used). So instead of hard coding the value we left the variable. @ThomThomson can you confirm?

src/plugins/presentation_util/public/components/controls/control_group/control_group.scss

andreadelrio · 2021-09-29T21:37:28Z

Regarding the Add control form, DeFazio brought up a good point which I think is worth considering. What if we reorder the fields so that Title comes after Index pattern and Field and we autofill Title with the value for Field. Users would still be free to edit that Title but if they're happy with the default it would save them time and be convenient.

Also, do we want to adjust the title of this flyout so that it is Add control for the creation flow and Manage/Edit control for the edit flow?

ThomThomson · 2021-09-29T23:14:13Z

@ryankeairns for the issue of sorting selected items to the top, I think we need to holistically consider it for the Options List. I have created a follow up issue, which I will address in another PR. The issue includes some of the items we talked about before with selections, and can use your input!

@andreadelrio:
Updating the titles of the management flyouts sounds like a great idea! In the future, we should also consider including the type of embeddable which is being edited, since this is set up to be able to handle multiple different types. So it would eventually read Create / Manage Options List Control

Moving the title to the bottom is a great idea UX-wise. Unfortunately, it will take some engineering effort, since the title is part of the generic editor, and the field name is selected in the editor for the specific input control type. That said, the UX benefits far outweigh the challenges, so it would be an ideal candidate for a follow-up PR.

…itDragDrop

Co-authored-by: Andrea Del Rio <[email protected]>

ThomThomson · 2021-10-04T14:27:47Z

@elasticmachine merge upstream

kibanamachine · 2021-10-04T14:27:48Z

merge conflict between base and head

…itDragDrop

ThomThomson · 2021-10-04T14:55:41Z

@elasticmachine merge upstream

…son/kibana into controls/dndKitDragDrop

vadimkibana

AppServices change LGTM.

Clean up flyouts

kibanamachine · 2021-10-04T23:48:04Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`presentationUtil`	119	121	+2

Async chunks

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

id	before	after	diff
`presentationUtil`	43.1KB	43.1KB	+6.0B

Page load bundle

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

id	before	after	diff
`embeddable`	70.6KB	70.6KB	+18.0B
`presentationUtil`	43.3KB	43.6KB	+351.0B
total			+369.0B

Unknown metric groups

References to deprecated APIs

id	before	after	diff
`presentationUtil`	13	4	-9

History

💔 Build #157772 failed e90f201
💛 Build #157590 was flaky 8a99e89
💔 Build #157583 failed 279741c
💚 Build #157131 succeeded 75d8fb3
💛 Build #156611 was flaky 08f8c71

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

ryankeairns

Thanks for the changes and for creating follow-up issues.

The border radius still looks a little off but, as Andrea and I discussed, we can do some final polish work once we see this within the Kibana context.

kibanamachine · 2021-10-07T16:34:59Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 111065 or prevent reminders by adding the backport:skip label.

ThomThomson and others added 14 commits August 12, 2021 15:00

added dndKit to package.json

961871f

Merge branch 'master' of github.com:elastic/kibana into controls/dndK…

82dc233

…itDragDrop

fix package json

672b4b2

Control group editing experience V2

0cdf909

Merge branch 'master' of github.com:elastic/kibana into controls/dndK…

36e21df

…itDragDrop

built control group embeddable, stubbed controls service, stubbed add…

1cfb999

…ing and removing controls

Control group embeddable creation and deletion working. Inline editin…

121c104

…g stub

Management, validation, restore previous input on cancel

e50493c

Fix types, fix file capitilization, reenable set all widths function

09236b5

working on clones

983229c

Merge branch 'master' of github.com:elastic/kibana into controls/dndK…

93344cb

…itDragDrop

fix truncation

695a657

renamed element

aef2049

Fix i18n, package.json merge conflict, fix embeddable parenting.

cdb50c1

ThomThomson force-pushed the controls/dndKitDragDrop branch from 3085e6b to cdb50c1 Compare September 21, 2021 22:07

ThomThomson and others added 2 commits September 21, 2021 16:59

Reorganize controls file structure

6488ebd

small style changes

7231eca

ThomThomson marked this pull request as ready for review September 22, 2021 14:50

ThomThomson requested review from a team as code owners September 22, 2021 14:50

ThomThomson changed the title ~~[Controls] Control Frame Layout Management Experience V2~~ [Controls] Control Group Embeddable and Management Experience Sep 22, 2021

ThomThomson mentioned this pull request Sep 22, 2021

[Controls] Control Frame Layout Management Design #109138

Closed

poffdeluxe approved these changes Sep 29, 2021

View reviewed changes

ryankeairns reviewed Sep 29, 2021

View reviewed changes

andreadelrio reviewed Sep 29, 2021

View reviewed changes

src/plugins/presentation_util/public/components/controls/control_group/control_group.scss Outdated Show resolved Hide resolved

andreadelrio reviewed Sep 29, 2021

View reviewed changes

src/plugins/presentation_util/public/components/controls/control_group/control_group.scss Outdated Show resolved Hide resolved

Id updates from review feedback

17a64e5

ThomThomson and others added 2 commits September 29, 2021 19:14

Merge branch 'master' of github.com:elastic/kibana into controls/dndK…

08f8c71

…itDragDrop

Apply suggestions from code review

75d8fb3

Co-authored-by: Andrea Del Rio <[email protected]>

Merge branch 'master' of github.com:elastic/kibana into controls/dndK…

279741c

…itDragDrop

kibanamachine and others added 3 commits October 4, 2021 10:55

Merge branch 'master' into controls/dndKitDragDrop

930e551

fix package.json

c8a0b53

Merge branch 'controls/dndKitDragDrop' of https://github.com/ThomThom…

8a99e89

…son/kibana into controls/dndKitDragDrop

vadimkibana approved these changes Oct 4, 2021

View reviewed changes

andreadelrio and others added 3 commits October 4, 2021 13:20

clean up flyouts

2bec309

Merge pull request #12 from andreadelrio/dndFlyouts

e90f201

Clean up flyouts

Type fixes

b7a328f

ryankeairns self-requested a review October 5, 2021 16:30

ryankeairns approved these changes Oct 5, 2021

View reviewed changes

ThomThomson merged commit ac39f94 into elastic:master Oct 5, 2021

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 7, 2021

ThomThomson added the backport:skip This commit does not require backporting label Oct 7, 2021

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Oct 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Controls] Control Group Embeddable and Management Experience #111065

[Controls] Control Group Embeddable and Management Experience #111065

ThomThomson commented Sep 2, 2021 •

edited

Loading

mdefazio commented Sep 22, 2021

poffdeluxe left a comment

poffdeluxe Sep 29, 2021

ThomThomson Sep 29, 2021

ryankeairns left a comment

ryankeairns Sep 29, 2021

ryankeairns commented Sep 29, 2021

andreadelrio commented Sep 29, 2021

Mismatched border-radius

andreadelrio commented Sep 29, 2021 •

edited

Loading

ThomThomson commented Sep 29, 2021

ThomThomson commented Oct 4, 2021

kibanamachine commented Oct 4, 2021

ThomThomson commented Oct 4, 2021

vadimkibana left a comment

kibanamachine commented Oct 4, 2021

References to deprecated APIs

ryankeairns left a comment

kibanamachine commented Oct 7, 2021

[Controls] Control Group Embeddable and Management Experience #111065

[Controls] Control Group Embeddable and Management Experience #111065

Conversation

ThomThomson commented Sep 2, 2021 • edited Loading

Summary

Todo next:

mdefazio commented Sep 22, 2021

poffdeluxe left a comment

Choose a reason for hiding this comment

poffdeluxe Sep 29, 2021

Choose a reason for hiding this comment

ThomThomson Sep 29, 2021

Choose a reason for hiding this comment

ryankeairns left a comment

Choose a reason for hiding this comment

Mismatched border-radius

Consider sorting selected items to the top

ryankeairns Sep 29, 2021

Choose a reason for hiding this comment

ryankeairns commented Sep 29, 2021

andreadelrio commented Sep 29, 2021

Mismatched border-radius

andreadelrio commented Sep 29, 2021 • edited Loading

ThomThomson commented Sep 29, 2021

ThomThomson commented Oct 4, 2021

kibanamachine commented Oct 4, 2021

ThomThomson commented Oct 4, 2021

vadimkibana left a comment

Choose a reason for hiding this comment

kibanamachine commented Oct 4, 2021

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

References to deprecated APIs

History

ryankeairns left a comment

Choose a reason for hiding this comment

kibanamachine commented Oct 7, 2021

ThomThomson commented Sep 2, 2021 •

edited

Loading

andreadelrio commented Sep 29, 2021 •

edited

Loading