Update sprite handling #308
Conversation
BREAKING CHANGE: use geostyler-style Sprite instead of string syntax
KaiVolland
force-pushed
the
sprite-support
branch
from
85790d3 to
9d883be
Compare
KaiVolland
changed the title
|
I created an issue for refactoring as coding/reviewing is already getting difficult: #309 |
There was a problem hiding this comment.
I'm not sure, if I like moving the metadata to a class member. IMHO, this only introduces not needed state handling, just for the benefit of not passing an additional argument to the methods.
I agree that this parser needs some refactoring, but I don't think that this approach is the solution to it.
| const { output: geoStylerStyle } = await styleParser.readStyle(mb_line_simpleline); | ||
| expect(geoStylerStyle).toBeDefined(); | ||
| expect(geoStylerStyle).toEqual(line_simpleline); | ||
| }); | ||
|
|
||
| it('can read a mapbox Line style with fill pattern', async () => { | ||
| expect.assertions(2); | ||
| mockFetchResult({ |
There was a problem hiding this comment.
Does it make sense to put this into a beforeEach?
There was a problem hiding this comment.
It is only needed for certain tests, so I would say it is not needed in a beforeEach.
| @@ -204,8 +230,16 @@ export class MapboxStyleParser implements StyleParser<Omit<MbStyle, 'sources'>> | |||
| 'geostyler:ref': GeoStylerRef; | |||
| }; | |||
|
|
|||
| private gsMetadata: { | |||
There was a problem hiding this comment.
Can we move this into a separate PR for better code review?
There was a problem hiding this comment.
I'd prefer to refactor this in a followup. See #309
Co-authored-by: Jan Suleiman <[email protected]>
|
🎉 This PR is included in version 5.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This updates the handling of mapbox sprites to make use of the geostyler
Sprite.❗ breaking change