[Control Group] Add Button & Minimal Editor Settings #151161

ThomThomson · 2023-02-14T15:56:43Z

Summary

Adds an "add" button that shows up to the right of the Control Group if configured. Also adds a system for fetching settings from consumers, and adds settings that can hide or show pieces of the Control editor flyout.

Also adds an example showing how to use these new systems.

…of control editor.

elasticmachine · 2023-02-15T18:13:57Z

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

andreadelrio · 2023-02-15T18:22:09Z

@ThomThomson could the showAddButton prop take any React node in case consumers want to pass a different type of button or a button with text instead of a EuiButtonIcon?

ThomThomson · 2023-02-15T18:51:27Z

@andreadelrio that very much depends on how flexible we want this button to be. As I see it, there are currently two options to get this add dialog to show up:

pass in the showAddButton prop as true. This will show the default design of the button in the default location.
On click of any button with any design, placed in any location call controlGroupApi.openAddDataControlFlyout from the control group API.

Allowing showAddButton to take any react node would add a third option:

Pass in a react node to show any button in the default location.

Do you think this third option would be used often? I think it might be better to ensure that the default design / location is ideal for the largest number of use-cases for consistency, but to leave that workaround using the API open and accessible.

andreadelrio · 2023-02-15T19:43:26Z

@andreadelrio that very much depends on how flexible we want this button to be. As I see it, there are currently two options to get this add dialog to show up:

pass in the showAddButton prop as true. This will show the default design of the button in the default location.

On click of any button with any design, placed in any location call controlGroupApi.openAddDataControlFlyout from the control group API.

Allowing showAddButton to take any react node would add a third option:

Pass in a react node to show any button in the default location.

Do you think this third option would be used often? I think it might be better to ensure that the default design / location is ideal for the largest number of use-cases for consistency, but to leave that workaround using the API open and accessible.

I agree. Let's leave it as-is then. Consumers that want to use the default design/location can opt for (1), otherwise they can do (2).

ThomThomson · 2023-02-15T20:22:18Z

Here is the default button design:

Thanks for the guidance @andreadelrio!

Heenawter

Code review + tested new example + tested Maps timeslider. Everything worked great - just left one tiny nit about the new button 👍

Heenawter · 2023-02-15T23:28:48Z

src/plugins/controls/public/control_group/component/control_group_component.tsx

+                <EuiButtonIcon
+                  size="s"
+                  iconSize="m"
+                  display="base"
+                  iconType={'plusInCircle'}
+                  onClick={() => controlGroup.openAddDataControlFlyout()}
+                />


nit: We should probably add an aria-label here, since it's an icon-only button.

Ah, good call! Added!

Heenawter · 2023-02-15T23:36:51Z

src/plugins/controls/public/control_group/editor/control_editor.tsx

@@ -99,6 +100,10 @@ export const ControlEditor = ({
    dataViews: { getIdsWithTitle, getDefaultId, get },
    controls: { getControlFactory },
  } = pluginServices.getServices();
+
+  const { useEmbeddableSelector: select } = useControlGroupContainerContext();
+  const editorConfig = select((state) => state.componentState.editorConfig);


Don't need to change this in this PR, but we should probably structure all the different hide<...> options list settings the same way... i.e. something like:

optionsListConfig: { hideActionBar?: boolean, hideExclude?: boolean, ... }

I think that would be a whole lot cleaner than it currently is 👍

That's a good call - likely we should change that! Especially because it's only used via code. Might do that in a follow--up!

jennypavlova

Infrastructure UI (Hosts View) changes look good! We don't use the edit functionality at the moment and everything works as expected. It could be interesting for us to try the edit functionality maybe in the future.

…tonAndCustomizableEditor

kibana-ci · 2023-02-22T18:02:53Z

💚 Build Succeeded

Buildkite Build
Commit: b703df0

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`controls`	266	268	+2

Async chunks

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

id	before	after	diff
`controls`	178.4KB	179.2KB	+772.0B
`infra`	1.3MB	1.3MB	+18.0B
`maps`	2.7MB	2.7MB	+24.0B
`securitySolution`	13.8MB	13.8MB	+18.0B
total			+832.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`controls`	9	11	+2

Page load bundle

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

id	before	after	diff
`controls`	32.1KB	32.1KB	+46.0B

Unknown metric groups

API count

id	before	after	diff
`controls`	270	272	+2

History

💚 Build #108395 succeeded fd520bb
💛 Build #108088 was flaky fb5e1cd
💔 Build #107961 failed 2f4ba06

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

logeekal

Functionality working great.. But in security solution, I am seeing a little lag ( see video below ). Since in your example it is running fine, it could be something I am doing wrong or maybe because of number of fields in the dataview mapping.

Screen.Recording.2023-02-23.at.13.26.35.mov

logeekal · 2023-02-23T11:47:12Z

src/plugins/controls/public/control_group/types.ts

+export interface ControlGroupSettings {
+  showAddButton?: boolean;
+  staticDataViewId?: string;
+  editorConfig?: {


logeekal · 2023-02-23T11:51:31Z

src/plugins/controls/public/control_group/editor/open_add_data_control_flyout.tsx

+            updateTitle={(newTitle) => (controlInput.title = newTitle)}
+            updateWidth={(defaultControlWidth) => this.updateInput({ defaultControlWidth })}
+            updateGrow={(defaultControlGrow: boolean) => this.updateInput({ defaultControlGrow })}
+            onSave={(type) => {


nit: should it be a useCallback function?

I think that useCallback is not required in this case due to openAddDataControlFlyout being an imperative function call and not a react component, so it gets rendered once as part of the useMountPoint call, and is in a different react tree.

ThomThomson · 2023-02-23T15:13:32Z

@logeekal, thanks for the review! For the performance issue, I believe it is indeed the number of fields in the data view. In #151231, I updated the field list to use EUI Selectable which is virtualized and should support many more fields without creating browser lag.

nreese

kibana-gis changes LGTM
code review

Add button to control group. Add settings to show only certain parts …

2f4ba06

…of control editor.

fix types

fb5e1cd

ThomThomson marked this pull request as ready for review February 15, 2023 18:13

ThomThomson requested review from a team as code owners February 15, 2023 18:13

jennypavlova self-requested a review February 15, 2023 18:41

update default styling for add button

fd520bb

Heenawter self-requested a review February 15, 2023 22:05

Heenawter approved these changes Feb 15, 2023

View reviewed changes

jennypavlova approved these changes Feb 16, 2023

View reviewed changes

ThomThomson added 2 commits February 22, 2023 10:17

Merge remote-tracking branch 'upstream/main' into controlGroup/addBut…

cdfb144

…tonAndCustomizableEditor

add aria label

b703df0

logeekal approved these changes Feb 23, 2023

View reviewed changes

nreese approved these changes Feb 23, 2023

View reviewed changes

ThomThomson merged commit f77f924 into elastic:main Feb 23, 2023

kibanamachine added the backport:skip This commit does not require backporting label Feb 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Control Group] Add Button & Minimal Editor Settings #151161

[Control Group] Add Button & Minimal Editor Settings #151161

ThomThomson commented Feb 14, 2023 •

edited

Loading

elasticmachine commented Feb 15, 2023

andreadelrio commented Feb 15, 2023

ThomThomson commented Feb 15, 2023

andreadelrio commented Feb 15, 2023

ThomThomson commented Feb 15, 2023

Heenawter left a comment •

edited

Loading

Heenawter Feb 15, 2023

ThomThomson Feb 22, 2023

Heenawter Feb 15, 2023

ThomThomson Feb 22, 2023

jennypavlova left a comment

kibana-ci commented Feb 22, 2023

API count

logeekal left a comment •

edited

Loading

logeekal Feb 23, 2023

logeekal Feb 23, 2023 •

edited

Loading

ThomThomson Feb 23, 2023

ThomThomson commented Feb 23, 2023 •

edited

Loading

nreese left a comment

[Control Group] Add Button & Minimal Editor Settings #151161

[Control Group] Add Button & Minimal Editor Settings #151161

Conversation

ThomThomson commented Feb 14, 2023 • edited Loading

Summary

elasticmachine commented Feb 15, 2023

andreadelrio commented Feb 15, 2023

ThomThomson commented Feb 15, 2023

andreadelrio commented Feb 15, 2023

ThomThomson commented Feb 15, 2023

Heenawter left a comment • edited Loading

Choose a reason for hiding this comment

Heenawter Feb 15, 2023

Choose a reason for hiding this comment

ThomThomson Feb 22, 2023

Choose a reason for hiding this comment

Heenawter Feb 15, 2023

Choose a reason for hiding this comment

ThomThomson Feb 22, 2023

Choose a reason for hiding this comment

jennypavlova left a comment

Choose a reason for hiding this comment

kibana-ci commented Feb 22, 2023

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

History

logeekal left a comment • edited Loading

Choose a reason for hiding this comment

logeekal Feb 23, 2023

Choose a reason for hiding this comment

logeekal Feb 23, 2023 • edited Loading

Choose a reason for hiding this comment

ThomThomson Feb 23, 2023

Choose a reason for hiding this comment

ThomThomson commented Feb 23, 2023 • edited Loading

nreese left a comment

Choose a reason for hiding this comment

ThomThomson commented Feb 14, 2023 •

edited

Loading

Heenawter left a comment •

edited

Loading

logeekal left a comment •

edited

Loading

logeekal Feb 23, 2023 •

edited

Loading

ThomThomson commented Feb 23, 2023 •

edited

Loading