[Maps] Support by value saved objects. #82486

nreese · 2020-11-03T19:38:56Z

This PR updates map embeddable and map application to support saved objects by value, allowing users to store map saved objects directly in dashboard without needed to create and maintain a separate saved object.

Testing instructions

Add dashboard.allowByValueEmbeddables: true in kibana.dev.yml
Create new dashboard
Use create new to add a panel and select maps.
Create your map and save with Save return
save your dashboard. Inspect dashboard saved object. Notice how map saved object is stored directly in dashboard.

elasticmachine · 2020-11-03T19:38:58Z

Pinging @elastic/kibana-gis (Team:Geo)

x-pack/plugins/maps/public/routing/routes/map_app/map_app.tsx

x-pack/plugins/maps/public/routing/store_operations.ts

ThomThomson

Tested locally in chrome.

Code LGTM. Everything looks super clean.

Adding to library, cloning panel, and unlinking all work flawlessly with by reference and by value panels!

One issue I found is that editing 'by reference' maps from a dashboard then hitting 'save and return' doesn't apply the changes properly. You need to press 'save to maps' to see your changes. Not totally sure what's going on there.

I also have some tiny UX nits that could maybe be addressed in a followup:

When editing a 'by reference' map from a dashboard, the button to the left of 'save and return' still says 'save to maps'. If you're editing though that means it's already been saved to maps. In Lens, in that case, the button reads 'save as'.
When editing a 'by reference' panel from dashboard I would say that it makes sense to have the 'maps' breadcrumb also present - so it would be Dashboard / Maps / My cool Map when editing from dashboard in by reference mode.
In both visualize and lens, new by value panels are saved with no title at all. Editing them shows something like Dashboard / Edit visualization in the breadcrumbs, and when you're in view mode on the dashboard, those blank titles just aren't shown. In Maps so far, by value maps are all named New Map, which means the users will have customize each panel's title, or hide the panel titles - else they will potentially have a bunch of 'New Map' on their dashboard.

Additionally, while testing I noticed that saving a map with filters applied doesn't actually apply when they are on a dashboard. This behaviour was the same in maps in 7.10 BC 4, and in 7.9.2 but I'm not sure if it's intentional or not. If not, maybe a separate issue.

Overall, this is a huge change, and will contribute a lot to the seamless feeling we hope to capture when we release Time to Visualize!

ThomThomson · 2020-11-05T19:45:31Z

x-pack/plugins/maps/public/routes/map_app/saved_map/saved_map.ts

+        description: '',
+      };
+    } else {
+      this._attributes = await getMapAttributeService().unwrapAttributes(this._mapEmbeddableInput);


This is looking really clean!

nreese · 2020-11-05T20:55:47Z

Additionally, while testing I noticed that saving a map with filters applied doesn't actually apply when they are on a dashboard. This behaviour was the same in maps in 7.10 BC 4, and in 7.9.2 but I'm not sure if it's intentional or not. If not, maybe a separate issue.

This is as expected behavior. Query, filters, and time saved with a map are not transferred to Dashboard

thomasneirynck

this is such a fun PR to play with. kibana and maps feel twice as fast.

the simplifcation in the Maps-code is impressive.

Any ideas on how to test? It'd make sense to add some functional test for the new flow.

x-pack/plugins/maps/public/embeddable/map_embeddable.tsx

x-pack/plugins/maps/public/routes/map_app/index.ts

x-pack/plugins/maps/public/routes/map_app/map_app_container.tsx

x-pack/plugins/maps/public/routes/map_app/map_app.tsx

x-pack/plugins/maps/public/routes/map_app/saved_map/saved_map.ts

x-pack/plugins/maps/public/lazy_load_bundle/index.ts

ThomThomson · 2020-11-06T22:18:58Z

@thomasneirynck - great timing with the question of adding functional tests. This PR #82441 was recently merged which should allow us to start writing tests with dashboard.allowByValueEmbeddables on.

nreese · 2020-11-09T14:35:10Z

@elasticmachine merge upstream

nreese · 2020-11-09T15:00:26Z

One issue I found is that editing 'by reference' maps from a dashboard then hitting 'save and return' doesn't apply the changes properly. You need to press 'save to maps' to see your changes. Not totally sure what's going on there.

Thanks for pointing this out. I had a logic error where a "by reference" map was calling wrapAttributes with useRefType as false so it was getting updated "by value". I have resolved the problem

nreese · 2020-11-09T16:35:34Z

@ThomThomson Thanks for all the great feedback on the little stuff like breadcrumbs, button labels, and page titles. These are great things to make consistent with Lens to provide a cohesive user experience. I have addressed the following.

When editing a 'by reference' map from a dashboard, the button to the left of 'save and return' still says 'save to maps'. If you're editing though that means it's already been saved to maps. In Lens, in that case, the button reads 'save as'.

Resolved. Added proper logic to set button label when "by value" or "by reference"

When editing a 'by reference' panel from dashboard I would say that it makes sense to have the 'maps' breadcrumb also present - so it would be Dashboard / Maps / My cool Map when editing from dashboard in by reference mode.

Resolved. Added proper logic to include "Maps" in path when "by reference"

In both visualize and lens, new by value panels are saved with no title at all. Editing them shows something like Dashboard / Edit visualization in the breadcrumbs, and when you're in view mode on the dashboard, those blank titles just aren't shown. In Maps so far, by value maps are all named New Map, which means the users will have customize each panel's title, or hide the panel titles - else they will potentially have a bunch of 'New Map' on their dashboard.

Resolved. Map title now defaults to empty string like lens. When you create a new map either from Maps or Dashboard, the page title is "Create". When you edit a map "by value", the page title is "Edit map". When you edit a map "by reference", the page title is the saved object title.

thomasneirynck

lgtm. So great, this flow feels really fast. Thanks for simplifying how the maps is bootstrapped in the page of the Maps-app and inside the embeddable.

ThomThomson

Tested again on Chrome. Everything works great! Thanks again for looking into those consistency nits.

LGTM

kibanamachine · 2020-11-10T19:02:07Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 45c2553

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`maps`	624	627	+3

Async chunks

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

id	before	after	diff
`apm`	3.1MB	3.1MB	+22.0B
`maps`	2.8MB	2.8MB	-6.2KB
`securitySolution`	7.8MB	7.8MB	-32.0B
`uptime`	983.1KB	983.1KB	-34.0B
total			-6.3KB

Page load bundle

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

id	before	after	diff
`canvas`	908.9KB	908.9KB	+22.0B
`maps`	155.6KB	150.7KB	-4.9KB
total			-4.9KB

History

💚 Build #86844 succeeded a5f4853
💔 Build #86820 failed 7921138
💚 Build #86768 succeeded 92feba6
💚 Build #86561 succeeded ace3e69
💚 Build #86557 succeeded c13676f

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

* [Maps] saved objects by value * inject references when unwrapping * clean up map embeddable initialize * use attribute service to load savedMap * clean up * remove clear ui since each route has its own store instance * save * update for API changes * pass input to stateTransfer * remove map saved object loader * remove unused store_operations * add saved objects to recently accessed * provide default description * break originatingApp connection when not returnToOrigin * clean up file structure * clean up adding help menu * use SavedMap in map_embeddable to remove dupicated load attributes code * clean up * restore imports * clean up breadcrumbs to match lens * fix check for duplicate title * tslint * make title map saved object attribute required * fix jest tests * fix logic for hasSaveAndReturnConfig to not show save and return button with new map and allowByValueEmbeddables disabled * tslint * fix functional test by triggering MapApp render after save * rename map_app_container to map_page * move MapApp and redux connector into folder * review feedback * use MAP_PATH constant * update by reference saved object on save and return * cleanup breadcrumbs and title * properly handle deleted map saved objects * tslint cleanup Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* [Maps] saved objects by value * inject references when unwrapping * clean up map embeddable initialize * use attribute service to load savedMap * clean up * remove clear ui since each route has its own store instance * save * update for API changes * pass input to stateTransfer * remove map saved object loader * remove unused store_operations * add saved objects to recently accessed * provide default description * break originatingApp connection when not returnToOrigin * clean up file structure * clean up adding help menu * use SavedMap in map_embeddable to remove dupicated load attributes code * clean up * restore imports * clean up breadcrumbs to match lens * fix check for duplicate title * tslint * make title map saved object attribute required * fix jest tests * fix logic for hasSaveAndReturnConfig to not show save and return button with new map and allowByValueEmbeddables disabled * tslint * fix functional test by triggering MapApp render after save * rename map_app_container to map_page * move MapApp and redux connector into folder * review feedback * use MAP_PATH constant * update by reference saved object on save and return * cleanup breadcrumbs and title * properly handle deleted map saved objects * tslint cleanup Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

nreese added 16 commits October 26, 2020 15:45

[Maps] saved objects by value

3247f86

Merge branch 'master' of github.com:elastic/kibana into by_value

dd73db4

inject references when unwrapping

1c9b361

clean up map embeddable initialize

5169f9c

Merge branch 'master' of github.com:elastic/kibana into by_value

f7d2440

merge with master

84843fd

use attribute service to load savedMap

7491503

clean up

5d3e6e1

remove clear ui since each route has its own store instance

a4387e8

Merge branch 'master' of github.com:elastic/kibana into by_value

1a34a6a

save

bff1eda

Merge branch 'master' of github.com:elastic/kibana into by_value

5be9a64

update for API changes

aca8f6e

Merge branch 'master' of github.com:elastic/kibana into by_value

f46093b

pass input to stateTransfer

aa1642e

remove map saved object loader

41294ca

nreese added release_note:enhancement WIP Work in progress [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 v7.11.0 labels Nov 3, 2020

nreese requested a review from a team as a code owner November 3, 2020 19:38

nreese added 3 commits November 3, 2020 12:42

remove unused store_operations

b7bf3be

add saved objects to recently accessed

eec85fd

provide default description

37e3372

nreese commented Nov 3, 2020

View reviewed changes

x-pack/plugins/maps/public/routing/routes/map_app/map_app.tsx Show resolved Hide resolved

nreese commented Nov 3, 2020

View reviewed changes

x-pack/plugins/maps/public/routing/store_operations.ts Show resolved Hide resolved

nreese added 2 commits November 3, 2020 15:25

break originatingApp connection when not returnToOrigin

5e21aec

clean up file structure

a9c3c10

fix functional test by triggering MapApp render after save

7752295

ThomThomson reviewed Nov 5, 2020

View reviewed changes

Merge branch 'master' of github.com:elastic/kibana into by_value

afd5375

thomasneirynck reviewed Nov 6, 2020

View reviewed changes

rename map_app_container to map_page

5261de4

nreese added 3 commits November 6, 2020 15:35

move MapApp and redux connector into folder

c13676f

review feedback

58b4432

use MAP_PATH constant

ace3e69

nreese requested a review from thomasneirynck November 6, 2020 22:55

kibanamachine and others added 2 commits November 9, 2020 09:35

Merge branch 'master' into by_value

92feba6

update by reference saved object on save and return

bd83e0a

cleanup breadcrumbs and title

7921138

nreese added 2 commits November 9, 2020 10:01

properly handle deleted map saved objects

8c11c29

tslint cleanup

a5f4853

nreese requested a review from ThomThomson November 9, 2020 17:10

thomasneirynck approved these changes Nov 10, 2020

View reviewed changes

ThomThomson approved these changes Nov 10, 2020

View reviewed changes

merge with master

45c2553

nreese merged commit 104f9aa into elastic:master Nov 10, 2020

nreese mentioned this pull request Nov 10, 2020

[7.x] [Maps] Support by value saved objects. (#82486) #83091

Merged

nreese mentioned this pull request Nov 19, 2020

[Dashboard] Prompt Visualize users to start with Dashboard #79925

Closed

kindsun mentioned this pull request Sep 9, 2021

[Maps] Add handling for shareable saved objects #111195

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Maps] Support by value saved objects. #82486

[Maps] Support by value saved objects. #82486

nreese commented Nov 3, 2020 •

edited

Loading

elasticmachine commented Nov 3, 2020

ThomThomson left a comment

ThomThomson Nov 5, 2020

nreese commented Nov 5, 2020

thomasneirynck left a comment •

edited

Loading

ThomThomson commented Nov 6, 2020 •

edited

Loading

nreese commented Nov 9, 2020

nreese commented Nov 9, 2020 •

edited

Loading

nreese commented Nov 9, 2020 •

edited

Loading

thomasneirynck left a comment

ThomThomson left a comment

kibanamachine commented Nov 10, 2020

[Maps] Support by value saved objects. #82486

[Maps] Support by value saved objects. #82486

Conversation

nreese commented Nov 3, 2020 • edited Loading

elasticmachine commented Nov 3, 2020

ThomThomson left a comment

Choose a reason for hiding this comment

ThomThomson Nov 5, 2020

Choose a reason for hiding this comment

nreese commented Nov 5, 2020

thomasneirynck left a comment • edited Loading

Choose a reason for hiding this comment

ThomThomson commented Nov 6, 2020 • edited Loading

nreese commented Nov 9, 2020

nreese commented Nov 9, 2020 • edited Loading

nreese commented Nov 9, 2020 • edited Loading

thomasneirynck left a comment

Choose a reason for hiding this comment

ThomThomson left a comment

Choose a reason for hiding this comment

kibanamachine commented Nov 10, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

nreese commented Nov 3, 2020 •

edited

Loading

thomasneirynck left a comment •

edited

Loading

ThomThomson commented Nov 6, 2020 •

edited

Loading

nreese commented Nov 9, 2020 •

edited

Loading

nreese commented Nov 9, 2020 •

edited

Loading