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

Layer container interface #132

Merged
merged 1 commit into from
Apr 3, 2016
Merged

Layer container interface #132

merged 1 commit into from
Apr 3, 2016

Conversation

boromisp
Copy link
Contributor

@boromisp boromisp commented Apr 1, 2016

Edit (2016-04-02):
Split the LayersControl code into a new branch, rebased to d057013, removed the built files, and filterd out the layerContainer prop everywhere from the layer options objects (same as the map prop).

This is a two-part PR.

The first part is the layerContainer prop.

Until now a MapLayer could receive either a map or a layerGroup prop. With this PR a MapLayer always receives both the map and the layerContainer props. The map is always the same, and the layerContainer is either the map or the old layerGroup.
This way all the map layers have access to the root Map instance, and other kind of layer containers can be introduced less intrusively.

The second is a LayersControl implementation for #87 that makes use of the layerContainer prop.

This uses a HoC for the child layers, and acts as a layers container.
I haven't tested it thoroughly, but for my use case it seems to be working.
The readme includes an example.

@boromisp
Copy link
Contributor Author

boromisp commented Apr 1, 2016

I'm not really happy with this yet, I'd much rather have a more declarative approach. Something like:

class MyMap extends Component {
    state = {
        selectedLayers: new Set(['tile-layer'])
    };

    handleLayersSelected = selectedLayers => this.setState({selectedLayers});

    render() {
        const { selectedLayers } = this.state;
        const { handleLayersSelected } = this;
        return (
             <Map>
                <LayersControl
                    baseLayers={{
                        'tile-layer': 'Tile layer label',
                        'some-other-layer': 'Other layer label'
                    }}
                    overlays={{
                        'optional-marker-layer': 'Markers...'
                    }}
                    selected={selectedLayers}
                    onChange={handleLayersSelected}
                />{selectedLayers.has('tile-layer') ?
                <TileLayer url={https://...} /> : null}{selectedLayers.has('some-other-layer') ?
                <OtherBaseLayer ... /> : null}{selectedLayers.has('optional-marker-layer') ?
                <LayerGroup>
                    {/* markers... */}
                </LayerGroup> : null}
            </Map>
        );
    }
}

This would require a custom LayersControl widget.

I put together a sample implementation, which includes:

  • a React-controlled L.Control,
  • a react-leaflet wrapper for that,
  • and the LayersControl re-implemented fully in react.

https://gist.github.com/boromisp/bdc8faf182d43bb95d784312a7eff0fc (Since I'm currently on Windows, these code snippets haven't been built or tested.)

But this works around leaflet rather then working with, so this might not be a good fit for this project.

@PaulLeCam
Copy link
Owner

Hi, thanks for this PR!

I like the "layer container" approach, would you mind adding this change only as a separate PR that could be merged before this one?

Regarding the layers control, I agree it would be nice to handle it directly as components, but I'm not sure the ControlledLayer HOC is the best way to go, as from my understanding of your code it would shadow the title and type props that could be used by the wrapped component.
I'll experiment a bit around this to try to come up with possible alternatives that could match both React and Leaflet APIs.

@boromisp
Copy link
Contributor Author

boromisp commented Apr 2, 2016

Hi, I updated the PR.

The ControlledLayer is the glue, that presents the LayersController as a kind of layerContainer to the MapLayer-s, and sends title, type and defaultAdded props to the LayersController for each MapLayer.

I haven't looked into it yet, but the leaflet 1.0 might bring some new concepts relevant to this, like custom map panes for layer ordering.

Leaflet/Leaflet#1742
http://leafletjs.com/examples/map-panes.html

@boromisp boromisp changed the title A more complete LayersControl implementation Layer container interface Apr 2, 2016
@PaulLeCam PaulLeCam merged commit 02ce664 into PaulLeCam:master Apr 3, 2016
@PaulLeCam
Copy link
Owner

Thanks!

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