Greater-London-Authority / ldn-viz-tools

https://greater-london-authority.github.io/ldn-viz-tools/
1 stars 0 forks source link

Add Maplibre Map component #170

Closed PaulioRandall closed 9 months ago

PaulioRandall commented 9 months ago

What does this change?

Adds a lightweight <Map> component for creating MapLibre maps.

Why?

So I can build maps.

How?

Two new components:

Related issues:

None.

Does this introduce new dependencies?

None.

How is it tested?

In storybook.

How is it documented?

In storybook.

Are light and dark themes considered?

Nope.

Is it complete?

This question is redundant. It's complete when we agree it's complete.

jamesscottbrown commented 9 months ago

The Map component takes an object of options as a prop.

Since the style attribute must be specified for anything to be rendered, it is required rather than optional and I don't think it belongs in an options object: either it should be a separate prop, or a default value should be used if options.style is undefined.

PaulioRandall commented 9 months ago

The Map component takes an object of options as a prop.

Since the style attribute must be specified for anything to be rendered, it is required rather than optional and I don't think it belongs in an options object: either it should be a separate prop, or a default value should be used if options.style is undefined.

While you're technically correct on the required/optional note, I crafted the <Map> component API so it's consistent with the maplibre_gl.Map class constructor API. I think this is the most readable and flexible approach. I 'm trying to deviate as little as possible to avoid confusion. It also allows us to reuse MapLibre documentation with the only exception being the container prop being set internally.

jamesscottbrown commented 9 months ago

While you're technically correct on the required/optional note, I crafted the <Map> component API so it's consistent with the maplibre_gl.Map class constructor API. I think this is the most readable and flexible approach. I 'm trying to deviate as little as possible to avoid confusion. It also allows us to reuse MapLibre documentation with the only exception being the container prop being set internally.

That is reasonable, but could we use os_light_vts as the default if style isn't specified?