feat(url): sync selected layers state to url #196
Conversation
|
|
||
| export function activeLayersSelector(state: State) { | ||
| const {main: mainId, compare: compareId} = selectedLayerIdsSelector(state); | ||
| export function activeLayersSelector( |
There was a problem hiding this comment.
The name of the selector is somehow misleading. It doesn't necessarily return the active layers, it just returns the layer details for the given layer ids. Maybe "layerDetailsSelector" would make more sense.
| const {main: mainId, compare: compareId} = selectedLayerIdsSelector(state); | ||
| export function activeLayersSelector( | ||
| state: State, | ||
| props: {[key: string]: string} | null |
There was a problem hiding this comment.
The type for the props could be more accurate.
{mainLayerId?: string, compareLayerId?: string}
would be nice.
Another nice solution would be to accept only one layer id here and return the layer details for the given id.
return state.layers.details[layerId]
This would make the selector more reusable.
Then we use the selector twice in the component, once with the mainLayerId and once with the compareLayerId.
|
|
||
| export function selectedLayersSelector(state: State): SelectedLayerItems { | ||
| export function selectedLayersSelector( |
There was a problem hiding this comment.
Same here... the name is misleading. The selector doesn't know which layers are selected, it just returns the layers depending on the props passed to the selector.
Maybe you can refactor this similar to the activeLayersSelector
closes #170