Skip to content

[drilldowns] require embeddables to opt into ON_OPEN_PANEL_MENU trigger#259637

Merged
nreese merged 7 commits intoelastic:mainfrom
nreese:issue_259590
Mar 26, 2026
Merged

[drilldowns] require embeddables to opt into ON_OPEN_PANEL_MENU trigger#259637
nreese merged 7 commits intoelastic:mainfrom
nreese:issue_259590

Conversation

@nreese
Copy link
Copy Markdown
Contributor

@nreese nreese commented Mar 25, 2026

Closes #259590

Before this PR, drilldown UI code added ON_OPEN_PANEL_MENU to an embeddable's supported triggers. This auto-magically allowed embeddables that implemented HasSupportedTriggers interface to support drilldowns that used ON_OPEN_PANEL_MENU trigger.

This auto-magic is causing problems with "as code" schemas. Embeddables register supported triggers on server during schema generation. Embeddables would not list ON_OPEN_PANEL_MENU in their supported triggers, allowing the UI to create drilldowns with triggers that are not contained in the schema.

This PR resolves the issue by removing the magic. Drilldown UI code no longer adds ON_OPEN_PANEL_MENU
to an embeddable's supported triggers. Instead, embeddables must explicitly include ON_OPEN_PANEL_MENU in supported triggers list.

PR also updates drilldownRegistry.getSchema to throw when no drilldowns match supporting triggers to give developers immediate feedback that drilldowns should not be included in schema if there is no overlap in supported triggers and drilldowns.

@nreese nreese changed the title [drilldowns] require embeddables to opt into all triggers [drilldowns] require embeddables to opt into ON_OPEN_PANEL_MENU trigger Mar 25, 2026
@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Mar 25, 2026

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Mar 25, 2026

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Mar 25, 2026

/ci

@nreese
Copy link
Copy Markdown
Contributor Author

nreese commented Mar 25, 2026

/ci

@nreese nreese marked this pull request as ready for review March 25, 2026 23:49
@nreese nreese requested review from a team as code owners March 25, 2026 23:49
@nreese nreese added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting v9.4.0 labels Mar 25, 2026
@elasticmachine
Copy link
Copy Markdown
Contributor

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

@botelastic botelastic bot added Feature:Drilldowns Embeddable panel Drilldowns Feature:Embedding Embedding content via iFrame labels Mar 25, 2026
@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
discover 1.6MB 1.6MB +20.0B
embeddable 42.3KB 42.2KB -28.0B
lens 2.0MB 2.0MB +61.0B
maps 3.2MB 3.2MB +21.0B
visualizations 332.1KB 332.2KB +21.0B
total +95.0B

Page load bundle

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

id before after diff
imageEmbeddable 5.7KB 5.7KB +21.0B

History

Copy link
Copy Markdown
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice simple changes, good magic removal. LGTM!

serializeState: () => serialize(savedObjectId$.getValue()),
getInspectorAdapters: () => searchEmbeddable.stateManager.inspectorAdapters.getValue(),
supportedTriggers: () => {
// No triggers are supported, but this is still required to pass the drilldown
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment should definitely have tipped us off that this was a bad code smell. Happy to see it removed!

Copy link
Copy Markdown
Contributor

@nickofthyme nickofthyme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review only

@nreese nreese merged commit 341779b into elastic:main Mar 26, 2026
18 checks passed
mbondyra added a commit to mbondyra/kibana that referenced this pull request Mar 26, 2026
…hanges

* commit '22bf09c82658b9511cbb2ad13f6dd29ad3526472': (21 commits)
  [Overlays System Flyout]: Support Child History (elastic#256339)
  KUA-Update event naming format and examples (elastic#259846)
  Fix pagerduty connector codeownership (elastic#259807)
  [Upgrade Assistant] Migrate Kibana deprecations flaky integration tests to unit tests (elastic#258981)
  [Upgrade Assistant] Migrate ES deprecations flaky integration tests to unit tests (elastic#258142)
  [Index Management] Migrate flaky integration tests to unit tests (elastic#258942)
  [Cases] Rename attachment id to saved object id (elastic#259158)
  [Entity Store] Change hash algo to sha256 (elastic#259453)
  [Security Solution] fixed enhanced security profile header showing for non-alert documents (elastic#259801)
  Update LaunchDarkly (main) (elastic#259008)
  [Discover] Add observability default ES|QL query (elastic#257268)
  Update dependency @redocly/cli to v2.21.1 (main) (elastic#259016)
  Gap reason detected (elastic#258231)
  [One Workflow] Historical executionContext and telemetry (elastic#258623)
  coderabbit: drop SigEvents (elastic#259863)
  [ci] Bump cypress disk (elastic#259861)
  Server timings (elastic#258915)
  Replace deprecated EUI icons in files owned by @elastic/kibana-cases (elastic#255633)
  [ci] Bump storybooks disk (elastic#259858)
  [drilldowns] require embeddables to opt into ON_OPEN_PANEL_MENU trigger (elastic#259637)
  ...
jeramysoucy pushed a commit to jeramysoucy/kibana that referenced this pull request Apr 1, 2026
…er (elastic#259637)

Closes elastic#259590

Before this PR, drilldown UI code added `ON_OPEN_PANEL_MENU` to an
embeddable's supported triggers. This auto-magically allowed embeddables
that implemented `HasSupportedTriggers` interface to support drilldowns
that used `ON_OPEN_PANEL_MENU` trigger.

This auto-magic is causing problems with "as code" schemas. Embeddables
register supported triggers on server during schema generation.
Embeddables would not list `ON_OPEN_PANEL_MENU` in their supported
triggers, allowing the UI to create drilldowns with triggers that are
not contained in the schema.

This PR resolves the issue by removing the magic. Drilldown UI code no
longer adds `ON_OPEN_PANEL_MENU`
to an embeddable's supported triggers. Instead, embeddables must
explicitly include `ON_OPEN_PANEL_MENU` in supported triggers list.

PR also updates `drilldownRegistry.getSchema` to throw when no
drilldowns match supporting triggers to give developers immediate
feedback that drilldowns should not be included in schema if there is no
overlap in supported triggers and drilldowns.

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
paulinashakirova pushed a commit to paulinashakirova/kibana that referenced this pull request Apr 2, 2026
…er (elastic#259637)

Closes elastic#259590

Before this PR, drilldown UI code added `ON_OPEN_PANEL_MENU` to an
embeddable's supported triggers. This auto-magically allowed embeddables
that implemented `HasSupportedTriggers` interface to support drilldowns
that used `ON_OPEN_PANEL_MENU` trigger.

This auto-magic is causing problems with "as code" schemas. Embeddables
register supported triggers on server during schema generation.
Embeddables would not list `ON_OPEN_PANEL_MENU` in their supported
triggers, allowing the UI to create drilldowns with triggers that are
not contained in the schema.

This PR resolves the issue by removing the magic. Drilldown UI code no
longer adds `ON_OPEN_PANEL_MENU`
to an embeddable's supported triggers. Instead, embeddables must
explicitly include `ON_OPEN_PANEL_MENU` in supported triggers list.

PR also updates `drilldownRegistry.getSchema` to throw when no
drilldowns match supporting triggers to give developers immediate
feedback that drilldowns should not be included in schema if there is no
overlap in supported triggers and drilldowns.

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
nreese added a commit that referenced this pull request Apr 9, 2026
…ers (#261018)

Follow up to #259637

Before this PR, drilldown UI code added ON_APPLY_FILTER to an
embeddable's supported triggers. This auto-magically allowed embeddables
that implemented HasSupportedTriggers interface to support drilldowns
that used ON_APPLY_FILTER trigger.

This auto-magic could cause problems with "as code" schemas. Embeddables
register supported triggers on server during schema generation.
Embeddables could not list ON_APPLY_FILTER in their supported triggers,
allowing the UI to create drilldowns with triggers that are not
contained in the schema. In practice this did not happen, but we want to
avoid the possibility for such a mistake.

This PR resolves the issue by removing the magic. Drilldown UI code no
longer adds ON_APPLY_FILTER
to an embeddable's supported triggers. Instead, embeddables must
explicitly includeON_APPLY_FILTER in supported triggers list.

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:Drilldowns Embeddable panel Drilldowns Feature:Embedding Embedding content via iFrame Project:Dashboards API release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

getDrilldownsSchema([]) can yield oneOf([]) and break dashboard REST validation

7 participants