Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

decouple url drilldown action from Embeddable framework #175930

Merged
merged 21 commits into from
Feb 5, 2024
Merged

decouple url drilldown action from Embeddable framework #175930

merged 21 commits into from
Feb 5, 2024

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Jan 30, 2024 •
edited
Loading

Part of #175138 and prerequisite for #174960

PR decouples Url drilldown action from Embeddable framework by migrating to sets of composable interfaces.

test instructions

  1. Create panel and add "Url drilldown"
  2. Verify context variables are populated with the same values when they were grabbed from embeddable.input and embeddable.output
    Screenshot 2024-01-31 at 2 11 20 PM

@nreese
Copy link
Contributor Author

nreese commented Jan 30, 2024

/ci

@nreese
Copy link
Contributor Author

nreese commented Jan 30, 2024

/ci

@nreese
Copy link
Contributor Author

nreese commented Jan 30, 2024

/ci

@nreese
Copy link
Contributor Author

nreese commented Jan 31, 2024

/ci

@nreese
Copy link
Contributor Author

nreese commented Jan 31, 2024

/ci

@nreese
Copy link
Contributor Author

nreese commented Jan 31, 2024

@elasticmachine merge upstream

@nreese
Copy link
Contributor Author

nreese commented Jan 31, 2024

/ci

@nreese
Copy link
Contributor Author

nreese commented Jan 31, 2024

/ci

@nreese nreese changed the title Migrate drilldown actions decouple url drilldown action from Embeddable framework Jan 31, 2024
@nreese
Copy link
Contributor Author

nreese commented Feb 1, 2024

/ci

@nreese nreese marked this pull request as ready for review February 1, 2024 14:03
@nreese nreese requested review from a team as code owners February 1, 2024 14:03
@nreese nreese added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas release_note:skip Skip the PR/issue when compiling release notes Feature:Embeddables Relating to the Embeddable system v8.13.0 labels Feb 1, 2024
@elasticmachine
Copy link
Contributor

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

@nreese nreese mentioned this pull request Feb 1, 2024
Closed
@ThomThomson ThomThomson self-requested a review February 1, 2024 20:25
@botelastic botelastic bot added Feature:Drilldowns Embeddable panel Drilldowns Feature:Embedding Embedding content via iFrame labels Feb 1, 2024
@ThomThomson ThomThomson added project:embeddableRebuild and removed Feature:Embedding Embedding content via iFrame labels Feb 2, 2024
ThomThomson
ThomThomson approved these changes Feb 2, 2024
View reviewed changes
Copy link
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.

Changes LGTM! Left one nit.

Tested locally in chrome by setting up a panel with all context in main and in the PR and comparing the output of a URL using all context variables.

Really great to see the shrinkage in the unit test code & complexity. That's a big part of why we're doing this, so it's great to see it paying dividends already. Nice work!

src/plugins/embeddable/public/lib/embeddables/compatibility/legacy_embeddable_to_api.ts Outdated Show resolved Hide resolved
src/plugins/embeddable/public/lib/triggers/triggers.ts
| ValueClickContext<T>
| MultiValueClickContext<T>
| RangeSelectContext<T>
export type ChartActionContext =
Copy link
Contributor

Choose a reason for hiding this comment

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

Great to see the template removed from this!

@botelastic botelastic bot added the Feature:Embedding Embedding content via iFrame label Feb 2, 2024
Dosant
Dosant approved these changes Feb 5, 2024
View reviewed changes
Copy link
Contributor

@Dosant Dosant left a comment

Choose a reason for hiding this comment

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

Lgtm, did some testing

@nreese
Copy link
Contributor Author

nreese commented Feb 5, 2024

@elasticmachine merge upstream

kqualters-elastic
kqualters-elastic approved these changes Feb 5, 2024
View reviewed changes
Copy link
Contributor

@kqualters-elastic kqualters-elastic left a comment

Choose a reason for hiding this comment

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

security solution changes 👍 lgtm

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

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
@kbn/ui-actions-browser 30 28 -2
embeddable 459 452 -7
uiActions 103 101 -2
total -11

Async chunks

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

id before after diff
dashboard 385.7KB 385.8KB +84.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 65.4KB 65.8KB +398.0B
urlDrilldown 15.5KB 15.7KB +244.0B
total +642.0B
Unknown metric groups

API count

id before after diff
@kbn/ui-actions-browser 44 42 -2
embeddable 564 557 -7
uiActions 149 147 -2
total -11

History

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

cc @nreese

@nreese nreese merged commit ca98633 into elastic:main Feb 5, 2024
37 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Feb 5, 2024
fkanout pushed a commit to fkanout/kibana that referenced this pull request Feb 7, 2024
@nreese @kibanamachine @fkanout
decouple url drilldown action from Embeddable framework (elastic#175930)
1ee8c5f 
Part of elastic#175138 and prerequisite
for elastic#174960

PR decouples Url drilldown action from Embeddable framework by migrating
to sets of composable interfaces.

### test instructions
1. Create panel and add "Url drilldown"
2. Verify `context` variables are populated with the same values when
they were grabbed from embeddable.input and embeddable.output
<img width="600" alt="Screenshot 2024-01-31 at 2 11 20 PM"
src="https://github.com/elastic/kibana/assets/373691/150f7a61-911f-4fb6-bf85-5c0481865e4b">

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
@nreese @kibanamachine @CoenWarmer
decouple url drilldown action from Embeddable framework (elastic#175930)
927f5d5 
Part of elastic#175138 and prerequisite
for elastic#174960

PR decouples Url drilldown action from Embeddable framework by migrating
to sets of composable interfaces.

### test instructions
1. Create panel and add "Url drilldown"
2. Verify `context` variables are populated with the same values when
they were grabbed from embeddable.input and embeddable.output
<img width="600" alt="Screenshot 2024-01-31 at 2 11 20 PM"
src="https://github.com/elastic/kibana/assets/373691/150f7a61-911f-4fb6-bf85-5c0481865e4b">

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
@ThomThomson ThomThomson mentioned this pull request Feb 22, 2024
Closed
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
@nreese @kibanamachine @fkanout
decouple url drilldown action from Embeddable framework (elastic#175930)
285ea9b 
Part of elastic#175138 and prerequisite
for elastic#174960

PR decouples Url drilldown action from Embeddable framework by migrating
to sets of composable interfaces.

### test instructions
1. Create panel and add "Url drilldown"
2. Verify `context` variables are populated with the same values when
they were grabbed from embeddable.input and embeddable.output
<img width="600" alt="Screenshot 2024-01-31 at 2 11 20 PM"
src="https://github.com/elastic/kibana/assets/373691/150f7a61-911f-4fb6-bf85-5c0481865e4b">

---------

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
Reviewers

@kqualters-elastic kqualters-elastic kqualters-elastic approved these changes

@ThomThomson ThomThomson ThomThomson approved these changes

@Dosant Dosant Dosant approved these changes

Assignees

@nreese nreese

Labels
backport:skip This commit does not require backporting Feature:Drilldowns Embeddable panel Drilldowns Feature:Embeddables Relating to the Embeddable system Feature:Embedding Embedding content via iFrame project:embeddableRebuild release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.13.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@nreese @elasticmachine @kibana-ci @Dosant @ThomThomson @kqualters-elastic @kibanamachine