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.

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

[Maps] Implement fields and bounds retrieval on GeoJsonFileSource #88294

Merged

Conversation

thomasneirynck
Copy link
Contributor

@thomasneirynck thomasneirynck commented Jan 13, 2021

Summary

Makes two additions:

  • add VectorSource#getFields implementaiton to GeoJsonFileSource. Does this by adding an internal __fields property. This enables dynamic styling for this source.
  • add bounds-retrieval to GeoJsonFileSource. This enables auto-fit (see below for lengthy explanation)

This blocks #88416. This PR would enable for ML to style the anomalies based on a custom color scheme, as well as get the auto-fitting behavior on startup.


Clients would use this like this:

const geojsonLayer: VectorLayerDescriptor = {
      type: 'VECTOR',
      id: 'foobar1',
      style: {
        properties: {
          fillColor: {
            type: STYLE_TYPE.DYNAMIC,
            options: {
              customColorRamp: [
                {
                  stop: 0,
                  color: '#FF0000',
                },
                {
                  stop: 1,
                  color: '#00FF00',
                },
              ],
              field: {
                name: 'foobarnumber',
                origin: FIELD_ORIGIN.SOURCE,
              },
              useCustomColorRamp: true,
            },
          },
        },
      },
      sourceDescriptor: {
        type: SOURCE_TYPES.GEOJSON_FILE,
        __fields: [
          {
            name: 'foobarnumber',
            type: 'number',
          },
          {
            name: 'foobarstring',
            type: 'string',
          },
        ],
        __featureCollection: {
          type: 'FeatureCollection',
          features: [
            {
              type: 'Feature',
              geometry: {
                type: 'Point',
                coordinates: [0, 0],
              },
              properties: {
                foobarnumber: 1,
                foobarstring: 'foobar',
              },
            },
          ],
        },
      },
    }

additional context

auto-fit-for-bounds does not work for non-ES sources. This is because auto-fitting implicitly relies on bounds-retrieval to be supported without data syncing

dispatch(addLayerWithoutDataSync(layerDescriptor));
.

This means that the fallback in vector_layer for sources which are not isBoundsAware does not work if syncData has not completed yet.

return getFeatureCollectionBounds(this._getSourceFeatureCollection(), this.hasJoins());

This approach works for 99% of use-cases, since any vector source is mostly backed by Elasticsearch. It is also important that auto-fit must work without fetching the data.

The GEOJSON_FILE source seems to be the exception.

  • It is only used transiently in Maps during the preview-phase of geojson-upload
  • It is also used by embeddables (e.g. Uptime, ML) to add layers with custom non-ES data

In order for auto-fitting to work before the data is loaded, the GEOJSON_FILE source needs to support bounds-retrieval.

Rather than changing the implementation of auto-fit, or expanding on the API, the easiest solution would be for GEOJSON_FILE to implement the getBounds method.

For maintainers

@thomasneirynck thomasneirynck added discuss [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation labels Jan 13, 2021
@thomasneirynck thomasneirynck added chore release_note:skip Skip the PR/issue when compiling release notes v7.12.0 v8.0.0 and removed discuss labels Jan 20, 2021
@thomasneirynck thomasneirynck changed the title [Maps] Implement bounds-retrieval on file source [Maps] Implement fields and bounds retrieval on GeoJsonFileSource Jan 20, 2021
@thomasneirynck thomasneirynck marked this pull request as ready for review January 26, 2021 00:56
@elasticmachine
Copy link
Contributor

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

@thomasneirynck thomasneirynck requested a review from nreese January 26, 2021 00:57
Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

LGTM - just one small comment about a more descriptive test name
code review

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
maps 684 685 +1

Async chunks

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

id before after diff
maps 2.9MB 2.9MB +3.6KB

History

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation release_note:skip Skip the PR/issue when compiling release notes v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants