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

[Embeddables Rebuild] Migrate Maps #174960

Closed
Tracked by #167429
ThomThomson opened this issue Jan 16, 2024 · 0 comments · Fixed by #178158
Closed
Tracked by #167429

[Embeddables Rebuild] Migrate Maps #174960

ThomThomson opened this issue Jan 16, 2024 · 0 comments · Fixed by #178158
Assignees
@nreese
Labels
Feature:Embeddables Relating to the Embeddable system impact:critical This issue should be addressed immediately due to a critical level of impact on the product. loe:x-large Extra Large Level of Effort project:embeddableRebuild Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas

Comments

@ThomThomson
Copy link
Contributor

ThomThomson commented Jan 16, 2024 •
edited
Loading

As part of the Embeddable refactor, we will need to transition the Maps Embeddable to the new framework.
@ThomThomson ThomThomson mentioned this issue Jan 16, 2024
Closed
@botelastic botelastic bot added the needs-team Issues missing a team label label Jan 16, 2024
@ThomThomson ThomThomson added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas impact:critical This issue should be addressed immediately due to a critical level of impact on the product. Feature:Embeddables Relating to the Embeddable system labels Jan 16, 2024
@botelastic botelastic bot removed the needs-team Issues missing a team label label Jan 16, 2024
@ThomThomson ThomThomson added the loe:large Large Level of Effort label Jan 16, 2024
@nreese nreese mentioned this issue Jan 22, 2024
Merged
nreese added a commit that referenced this issue Jan 25, 2024
@nreese @kibanamachine
[embeddable] decouple FILTER_BY_MAP_EXTENT action from Embeddable fra…
4565b1b 
…mework (#175262)

Part of #175138 and prerequisite
for #174960

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

### Inheritance to composition

Replace action context type with a type definition that is as narrow as
possible. How do you know which properties are needed? Look through the
action and find all usages of `embeddabe`. Add each usage to the
narrowed type definition.
```
// original action context type.
// Notice how type requires an Embeddable instance 
// when only a few properties from the embeddable are needed
export interface FilterByMapExtentActionContext {
  embeddable: Embeddable<FilterByMapExtentInput>;
}

// new action context type. 
// Notice how type explicitly states what is used and 
// is no longer tied to an Embeddable (even though an Embeddable will pass the type guard)
// Another point to highlight is use of 'Partial'. This indicates that API interfaces are not required
export type FilterByMapExtentActionApi = HasType &
  Partial<HasDisableTriggers & HasParentApi<HasType> & HasVisualizeConfig>;

// This is the new action context rewritten to inline types like HasType to make it clearer 
export type FilterByMapExtentActionApi = {
  type: string;
  disableTriggers?: boolean;
  parentApi?: {
    type: string
  };
  getVis?: () => Vis<VisParam>;
}
```

### Migrating embeddable.parent?.type
Action uses `embeddable.parent?.type` to customize the display name. For
dashboards the display name would be "Filter dashboards by map bounds",
otherwise the display name would be "Filter page by map bounds".

To access `parentApi`, we must:

1) `HasParentApi` interface and `apiHasParentApi` type guard already
exist at
`packages/presentation/presentation_publishing/interfaces/has_parent_api.ts`,
so we can just use those.
2) No compatibility changes are required since Embeddable already
exposes `parentApi`.
3) Add `Partial<HasParentApi<HasType>>` to action compatibility
definition. Using `Partial` because accessing `parentApi` is not
required.
4) Access value like `api.parentApi?.type`

### Migrating embeddable.getInput().disableTriggers
Action fails compatibility check when
`embeddable.getInput().disableTriggers` returns `true`.

To access `disableTriggers`, we must:

1) Define `HasDisableTriggers` interface and `apiHasDisableTriggers`
type guard in the package
`packages/presentation/presentation_publishing`. Adding interface and
type guard to generic package folder since `input.disableTriggers` is
not tied to any specific Embeddable implementation. *Naming convention:
Use prefix `has` since `disableTriggers` is static. Use prefix
`publishes` for dynamic values since value will be accesses via a
subscription.*
2) Add compatibility changes to
`src/plugins/embeddable/public/lib/embeddables/embeddable.tsx` so that
an Embeddable instance exposes `disableTriggers`.
3) Add `Partial<HasDisableTriggers>` to action compatibility definition.
Using `Partial` because accessing `disableTriggers` is not required.
4) Access value like `api.disableTriggers`

### Migrating embeddable.getVis()
Action uses `embeddable.getVis()` to pass compatibility check when
visualize embeddable is a region_map or tile_map

To access `getVis`, we must:

1) Define `HasVisualizeConfig` interface and `apiHasVisualizeConfig`
type guard in `src/plugins/visualizations/public/embeddable/interfaces`.
Adding interface to the visualize plugin since `getVis` is only
available for Visualization Embeddable.
2) No compatibility changes are required since Visualization Embeddable
already exposes `getVis` method.
3) Add `Partial<HasVisualizeConfig>` to action compatibility definition.
Using `Partial` because accessing `getVis` is not required.
4) Access value like `api.getVis()`

---------

Co-authored-by: kibanamachine <[email protected]>
@nreese nreese mentioned this issue Jan 25, 2024
Merged
nreese added a commit that referenced this issue Jan 29, 2024
@nreese @kibanamachine @ThomThomson
[Maps] decouple SYNCHRONIZE_MOVEMENT_ACTION from Embeddable framework (…
5b38614 
…#175642)

Part of #175138 and prerequisite
for #174960

PR decouples SYNCHRONIZE_MOVEMENT_ACTION from Embeddable framework by
migrating to sets of composable interfaces.

---------

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Devon Thomson <[email protected]>
@nreese nreese mentioned this issue Jan 31, 2024
Merged
nreese added a commit that referenced this issue Feb 5, 2024
@nreese @kibanamachine
decouple url drilldown action from Embeddable framework (#175930)
ca98633 
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
<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 <[email protected]>
fkanout pushed a commit to fkanout/kibana that referenced this issue 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 <[email protected]>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this issue Feb 15, 2024
@nreese @kibanamachine @CoenWarmer
[embeddable] decouple FILTER_BY_MAP_EXTENT action from Embeddable fra…
12fc3fa 
…mework (elastic#175262)

Part of elastic#175138 and prerequisite
for elastic#174960

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

### Inheritance to composition

Replace action context type with a type definition that is as narrow as
possible. How do you know which properties are needed? Look through the
action and find all usages of `embeddabe`. Add each usage to the
narrowed type definition.
```
// original action context type.
// Notice how type requires an Embeddable instance 
// when only a few properties from the embeddable are needed
export interface FilterByMapExtentActionContext {
  embeddable: Embeddable<FilterByMapExtentInput>;
}

// new action context type. 
// Notice how type explicitly states what is used and 
// is no longer tied to an Embeddable (even though an Embeddable will pass the type guard)
// Another point to highlight is use of 'Partial'. This indicates that API interfaces are not required
export type FilterByMapExtentActionApi = HasType &
  Partial<HasDisableTriggers & HasParentApi<HasType> & HasVisualizeConfig>;

// This is the new action context rewritten to inline types like HasType to make it clearer 
export type FilterByMapExtentActionApi = {
  type: string;
  disableTriggers?: boolean;
  parentApi?: {
    type: string
  };
  getVis?: () => Vis<VisParam>;
}
```

### Migrating embeddable.parent?.type
Action uses `embeddable.parent?.type` to customize the display name. For
dashboards the display name would be "Filter dashboards by map bounds",
otherwise the display name would be "Filter page by map bounds".

To access `parentApi`, we must:

1) `HasParentApi` interface and `apiHasParentApi` type guard already
exist at
`packages/presentation/presentation_publishing/interfaces/has_parent_api.ts`,
so we can just use those.
2) No compatibility changes are required since Embeddable already
exposes `parentApi`.
3) Add `Partial<HasParentApi<HasType>>` to action compatibility
definition. Using `Partial` because accessing `parentApi` is not
required.
4) Access value like `api.parentApi?.type`

### Migrating embeddable.getInput().disableTriggers
Action fails compatibility check when
`embeddable.getInput().disableTriggers` returns `true`.

To access `disableTriggers`, we must:

1) Define `HasDisableTriggers` interface and `apiHasDisableTriggers`
type guard in the package
`packages/presentation/presentation_publishing`. Adding interface and
type guard to generic package folder since `input.disableTriggers` is
not tied to any specific Embeddable implementation. *Naming convention:
Use prefix `has` since `disableTriggers` is static. Use prefix
`publishes` for dynamic values since value will be accesses via a
subscription.*
2) Add compatibility changes to
`src/plugins/embeddable/public/lib/embeddables/embeddable.tsx` so that
an Embeddable instance exposes `disableTriggers`.
3) Add `Partial<HasDisableTriggers>` to action compatibility definition.
Using `Partial` because accessing `disableTriggers` is not required.
4) Access value like `api.disableTriggers`

### Migrating embeddable.getVis()
Action uses `embeddable.getVis()` to pass compatibility check when
visualize embeddable is a region_map or tile_map

To access `getVis`, we must:

1) Define `HasVisualizeConfig` interface and `apiHasVisualizeConfig`
type guard in `src/plugins/visualizations/public/embeddable/interfaces`.
Adding interface to the visualize plugin since `getVis` is only
available for Visualization Embeddable.
2) No compatibility changes are required since Visualization Embeddable
already exposes `getVis` method.
3) Add `Partial<HasVisualizeConfig>` to action compatibility definition.
Using `Partial` because accessing `getVis` is not required.
4) Access value like `api.getVis()`

---------

Co-authored-by: kibanamachine <[email protected]>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this issue Feb 15, 2024
@nreese @kibanamachine
@ThomThomson @CoenWarmer
[Maps] decouple SYNCHRONIZE_MOVEMENT_ACTION from Embeddable framework (…
9a56c10 
…elastic#175642)

Part of elastic#175138 and prerequisite
for elastic#174960

PR decouples SYNCHRONIZE_MOVEMENT_ACTION from Embeddable framework by
migrating to sets of composable interfaces.

---------

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Devon Thomson <[email protected]>
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this issue 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 <[email protected]>
fkanout pushed a commit to fkanout/kibana that referenced this issue Mar 4, 2024
@nreese @kibanamachine
@ThomThomson @fkanout
[Maps] decouple SYNCHRONIZE_MOVEMENT_ACTION from Embeddable framework (…
fc2e906 
…elastic#175642)

Part of elastic#175138 and prerequisite
for elastic#174960

PR decouples SYNCHRONIZE_MOVEMENT_ACTION from Embeddable framework by
migrating to sets of composable interfaces.

---------

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Devon Thomson <[email protected]>
fkanout pushed a commit to fkanout/kibana that referenced this issue 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 <[email protected]>
@nreese nreese mentioned this issue Mar 11, 2024
Merged
This was referenced Apr 26, 2024
Merged
Closed
@nreese nreese mentioned this issue Apr 30, 2024
Merged
nreese added a commit that referenced this issue Apr 30, 2024
@nreese
rename SerializedMapState to ParsedMapStateJSON and SerializedUiState…
daf81a6 
… to ParsedUiStateJSON (#182185)

Part of #174960

#178158 creates a new type
`MapSerializeState`. This new type name is very confusing with existing
`SerializedMapState` type name. This PR renames `SerializedMapState` to
`ParsedMapStateJSON` to avoid this confusion. Breaking this change out
of #174960 to make things
clearer and avoid confusion during reviewing
#174960
This was referenced May 7, 2024
Merged
Merged
@nreese nreese added loe:x-large Extra Large Level of Effort and removed loe:large Large Level of Effort labels May 7, 2024
nickpeihl added a commit that referenced this issue May 9, 2024
@nickpeihl
Replace legacy Maps embeddable with MapComponent in APM GeoMap (#182753)
99e7bd3 
## Summary

Replaces the Maps embeddable with a new MapComponent in the APM
solution.

The Maps plugin now provides an easier to use MapComponent for
consumers. The legacy MapEmbeddable factory is also being removed as
part of #174960 which requires
making changes to consumers.


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
nickpeihl added a commit that referenced this issue May 9, 2024
@nickpeihl
Replace legacy MapEmbeddable with MapComponent in Obs UX solution (#1…
49a5d5e 
…82841)

## Summary

Replaces the Maps embeddable with a new MapComponent in the
Observability UX solution.

The Maps plugin now provides an easier to use MapComponent for
consumers. The legacy MapEmbeddable factory is also being removed as
part of #174960 which requires
making changes to consumers.


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
nreese added a commit that referenced this issue Jun 13, 2024
@nreese @kibanamachine @nickpeihl
React map embeddable (#178158)
3d419ec 
Closes #174960 and
#179677

PR refactors Map embeddable from legacy Embeddable class to react
embeddable.

---------

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Nick Peihl <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nreese nreese

Labels
Feature:Embeddables Relating to the Embeddable system impact:critical This issue should be addressed immediately due to a critical level of impact on the product. loe:x-large Extra Large Level of Effort project:embeddableRebuild Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

React map embeddable nreese/kibana
2 participants
@nreese @ThomThomson