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

Adding a MapboxLayer for creating a single layer from a Mapbox style #268

Closed
wants to merge 3 commits into from

Conversation

tschaub
Copy link
Member

@tschaub tschaub commented May 1, 2020

This adds a MapboxVectorLayer constructor that can be used like a normal OpenLayers layer. I wanted to make things work (where possible) with a style created with Mapbox studio and hosted on api.mapbox.com.

Example usage:

new Map({
  target: 'map',
  view: new View({
    center: [0, 0],
    zoom: 2
  }),
  layers: [
    new MapboxVectorLayer({
      styleURL: 'mapbox://styles/tschaub/cjh953yhw1...',
      accessToken:'pk.redacted'
    })
  ]
});

Where the styleURL and accessToken are copied from studio:
image

I'm leaving this as a draft pull request because I'm sure there are issues that I didn't consider. But the example works for me with a custom style and mapbox://styles/mapbox/bright-v9.

I was uncertain about the glyph URLs. Also wondering if something like the normalize*URL functions would be useful elsewhere in the library.

@tschaub
Copy link
Member Author

tschaub commented May 1, 2020

It looks like tsc is not properly understanding the inheritance...

@ahocevar
Copy link
Member

ahocevar commented May 2, 2020

Nice, thanks @tschaub! I think it should also be confirmed that the source type is vector.

Looking at this pull request, another nice API improvement could be a MapboxLayerGroup, inheriting from LayerGroup. That could contain all the layers that need to be created to represent the whole Mapbox Style, regardless of the source type. That would be similar to apply.

@ahocevar
Copy link
Member

ahocevar commented May 2, 2020

Thanks @tschaub for your continued effort on this. I just created openlayers/openlayers#10993, which will help to fix the typecheck issues we have in this pull request. I'm also currently working on a pull request here to fix the Typescript configuration to make inheritance from OpenLayers classes work.

@ahocevar
Copy link
Member

ahocevar commented May 2, 2020

Unfortunately, after fixing openlayers/openlayers#10993, we are running into a TypeScript issue (maybe microsoft/TypeScript#36830). For now, I think it makes sense to add a // @ts-ignore to the lines that make the type checking fail.

@ahocevar
Copy link
Member

ahocevar commented May 3, 2020

@tschaub If you rebase this pull request on top of master, type checking will work - and indicate that you're accessing protected methods of ol/source/Source#setState:

> tsc --project tsconfig-typecheck.json

src/MapboxVectorLayer.js:144:14 - error TS2445: Property 'setState' is protected and only accessible within class 'Source' and its subclasses.

144       source.setState(SourceState.READY);
                 ~~~~~~~~

src/MapboxVectorLayer.js:154:12 - error TS2445: Property 'setState' is protected and only accessible within class 'Source' and its subclasses.

154     source.setState(SourceState.ERROR);
               ~~~~~~~~


Found 2 errors.

Not sure how to best deal with that. Maybe add the MapboxVectorLayer to ol instead of ol-mapbox-style, or remove the @protected restriction in ol/source/Source#setState?

@tschaub
Copy link
Member Author

tschaub commented May 3, 2020

Thanks for making the TypeScript stuff work better. I do think this makes sense in the ol library, so I opened openlayers/openlayers#10996 for that (with rendering and a few unit tests).

@tschaub tschaub closed this May 3, 2020
@tschaub tschaub deleted the mapbox-layer branch May 3, 2020 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants