elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.6k stars 8.22k forks source link

[Dashboards API] Rename the `panels` key on control schema? #194757

Open nickpeihl opened 1 month ago

nickpeihl commented 1 month ago
          Why the rename of `explicitInput` to `embeddableConfig`. The controls are not embeddables. If we are renaming, how about just `control`?

_Originally posted by @nreese in https://github.com/elastic/kibana/pull/193067#discussion_r1781634413_

Since we are decoupling the Dashboards content management schema from the saved objects schema as part of #193067, perhaps we should take time to consider an alternate schema for controls.

The controlGroupInput.panelsJSON key in the dashboard saved object is similarly named to the top-level panelsJSON. However, the values of both objects are not really aligned. I'm wondering if consumers of the API would benefit if we differentiate the key names.

One consideration is rename the controlGroupInput.panels key to controlGroupInput.controls. We should maybe also consider changing controlGroupInput.panels.embeddableConfig to something else since (as far as users are concerned) controls are not really embeddables in the same way panels are (locked in place, can't resize by dragging, etc).

@Heenawter do you have thoughts?

elasticmachine commented 1 month ago

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

Heenawter commented 1 month ago

As @nreese pointed out, the new React controls are no longer embeddables - the only embeddable is the control group, and the control types are registered via a control-specific registry (similar to embeddables, but different)

One consideration is rename the controlGroupInput.panels key to controlGroupInput.controls

I think either one is fine - personally, I think I'm leaning towards controlGroupInput.controls... But I don't have a strong preference.

should maybe also consider changing controlGroupInput.panels.embeddableConfig

How about controlGroupInput.controls.controlConfig?

nreese commented 1 month ago

controlGroupInput.controls

+1

controlGroupInput.controls.controlConfig

+1

nickpeihl commented 3 weeks ago

After offline discussions, we are also renaming the panels[i].embeddableConfig property to panels[i].panelConfig in this effort.