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

[Maps] add initial location option that fits to data bounds #74583

Merged
merged 5 commits into from
Aug 11, 2020
Merged

[Maps] add initial location option that fits to data bounds #74583

merged 5 commits into from
Aug 11, 2020

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Aug 6, 2020

fixes #72480

This PR adds a new initial map location setting "Auto fit map to data bounds". When enabled, the map's initial location will be at a view to show all data for the query context.

Screen Shot 2020-08-06 at 1 04 31 PM

@nreese nreese added release_note:enhancement [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 v7.10.0 labels Aug 6, 2020
@nreese nreese requested a review from thomasneirynck August 6, 2020 19:06
@nreese nreese requested a review from a team as a code owner August 6, 2020 19:06
@elasticmachine
Copy link
Contributor

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

nreese
nreese commented Aug 6, 2020
View reviewed changes
x-pack/plugins/maps/public/actions/data_request_actions.ts
@@ -365,6 +366,9 @@ export function fitToDataBounds() {
}

if (!corners.length) {
if (onNoBounds) {
Copy link
Contributor Author

@nreese nreese Aug 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Added this callback to fix error with autofit to bounds where when there are no matching documents, sync all layers is not triggered because setGotoWithBounds is not triggered

@nreese
Copy link
Contributor Author

nreese commented Aug 10, 2020

@elasticmachine merge upstream

thomasneirynck
thomasneirynck reviewed Aug 10, 2020
View reviewed changes
Copy link
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

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

Thx great PR. Just small comment. Consider changing the map-settings of one of the saved maps in the functional tests to have this setting enable (e.g. document example), so we can verify this behavior in CI.

@nreese
Copy link
Contributor Author

nreese commented Aug 10, 2020 •
edited
Loading

Consider just returning the bounds or null, so the caller can just await on the result. This callback-API, but it's more convoluted than just calling a function and waiting for a return. (fwiw, i don't feel strongly).

The problem is that the method is a redux thunk. I do not believe thunks can return values. All state gets updated via dispatch so returns can just be a function (another thunk) or pure redux action (like { type: SET_MAP_SETTINGS, settings }) .

@nreese nreese requested a review from thomasneirynck August 10, 2020 18:42
@nreese
Copy link
Contributor Author

nreese commented Aug 10, 2020

Consider changing the map-settings of one of the saved maps in the functional tests to have this setting enable (e.g. document example), so we can verify this behavior in CI.

I have added a functional test case to verify initial auto fit to bounds on map load

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

async chunks size

id value diff baseline
maps 3.3MB +1.3KB 3.3MB

page load bundle size

id value diff baseline
maps 287.1KB +60.0B 287.1KB

History

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

thomasneirynck
thomasneirynck approved these changes Aug 11, 2020
View reviewed changes
Copy link
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

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

Awesome!

@nreese nreese merged commit 75b8a3c into elastic:master Aug 11, 2020
nreese added a commit to nreese/kibana that referenced this pull request Aug 11, 2020
@nreese @elasticmachine
[Maps] add initial location option that fits to data bounds (elastic#…
6430982 
…74583)

* [Maps] add initial location option that fits to data bounds

* update navigation_panel snapshot

* add functional test to ensure sync is called when auto fit to bounds with no data

* add functional test for auto fit to bounds on map load

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@nreese nreese mentioned this pull request Aug 11, 2020
Merged
gmmorris added a commit to gmmorris/kibana that referenced this pull request Aug 11, 2020
@gmmorris
Merge branch 'master' into actions/webhook-remove-header
1dbf2f2 
* master: (106 commits)
  [Functional Tests] Adds a wait time between setting the index pattern and the time field on TSVB (elastic#74736)
  [Lens] Add styling options for x and y axes on the settings popover (elastic#71829)
  [Maps] add initial location option that fits to data bounds (elastic#74583)
  theme function (elastic#73451)
  [data.ui.query] Write filters to query log from default editor. (elastic#74474)
  [data.search.SearchSource] Move some SearchSource dependencies to the server. (elastic#74607)
  [Canvas][tech-debt] Convert renderers (elastic#74134)
  [security solutions][lists] Adds end to end tests (elastic#74473)
  pluralized for occurrences vs occurrence (elastic#74564)
  Update links that pointed to CONTRIBUTING.md (elastic#74757)
  [Ingest pipelines] Implement tabs in processor flyout (elastic#74469)
  [Event log] Use Alerts client & Actions client when fetching these types of SOs (elastic#73257)
  Bump chalk to 4.1.0 (elastic#73397)
  Index pattern field list - transition away from extending array - introduce and use getAll() (elastic#74718)
  [SECURITY] Bugs css/inspect (elastic#74711)
  [telemetry] update README to downplay ui_metrics (elastic#74635)
  Fixed grammar (elastic#74725)
  [Telemetry][API Integration] size_in_bytes to be a number (elastic#74664)
  [ILM] Convert node details flyout to TS (elastic#73707)
  [Ingest Node Pipelines] Sentence-case processor names (elastic#74645)
  ...
nreese added a commit that referenced this pull request Aug 11, 2020
@nreese @elasticmachine
[Maps] add initial location option that fits to data bounds (#74583) (#…
81af59e 
…74776)

* [Maps] add initial location option that fits to data bounds

* update navigation_panel snapshot

* add functional test to ensure sync is called when auto fit to bounds with no data

* add functional test for auto fit to bounds on map load

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 12, 2020
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into ingest-pipeli…
bbbe9d2 
…nes/processor-forms-a-d

* 'master' of github.com:elastic/kibana: (25 commits)
  [ML] Removing full lodash library imports (elastic#74742)
  [Search] Server strategy example (elastic#71679)
  [Reporting] Fix and test for Listing of Reports (elastic#74453)
  [maps] fix drawing shapes (elastic#74689)
  [Resolver] Improve simulator. Add more click-through tests and panel tests. (elastic#74601)
  Deprecate schema-less specs in Vega (elastic#73805)
  [Security Solution] Rename Administration > Hosts subtab to Endpoints (elastic#74287)
  Timelion deprecation doc (elastic#74508)
  [Functional Tests] Adds a wait time between setting the index pattern and the time field on TSVB (elastic#74736)
  [Lens] Add styling options for x and y axes on the settings popover (elastic#71829)
  [Maps] add initial location option that fits to data bounds (elastic#74583)
  theme function (elastic#73451)
  [data.ui.query] Write filters to query log from default editor. (elastic#74474)
  [data.search.SearchSource] Move some SearchSource dependencies to the server. (elastic#74607)
  [Canvas][tech-debt] Convert renderers (elastic#74134)
  [security solutions][lists] Adds end to end tests (elastic#74473)
  pluralized for occurrences vs occurrence (elastic#74564)
  Update links that pointed to CONTRIBUTING.md (elastic#74757)
  [Ingest pipelines] Implement tabs in processor flyout (elastic#74469)
  [Event log] Use Alerts client & Actions client when fetching these types of SOs (elastic#73257)
  ...

# Conflicts:
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/field_components/text_editor.tsx
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/manage_processor_form.tsx
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/append.tsx
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/bytes.tsx
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/circle.tsx
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/common_fields/field_name_field.tsx
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/common_fields/ignore_missing_field.tsx
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/convert.tsx
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/csv.tsx
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/date.tsx
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/date_index_name.tsx
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/dissect.tsx
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/dot_expander.tsx
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/drop.tsx
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/index.ts
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_processors_editor/components/manage_processor_form/processors/shared.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thomasneirynck thomasneirynck thomasneirynck approved these changes

Assignees
No one assigned
Labels
[Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation release_note:enhancement v7.10.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Maps] add initial location option that fits to data bounds
4 participants
@nreese @elasticmachine @kibanamachine @thomasneirynck