From 1350c97aaeeaaf7f34aa1f53a13268117328238d Mon Sep 17 00:00:00 2001 From: Simon Seyock Date: Tue, 4 Jun 2024 17:31:17 +0200 Subject: [PATCH 1/2] fix: layerswitcher always shows next layer instead of current layer --- src/LayerSwitcher/LayerSwitcher.spec.tsx | 17 ++++---- src/LayerSwitcher/LayerSwitcher.tsx | 49 ++++++++++++------------ 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/src/LayerSwitcher/LayerSwitcher.spec.tsx b/src/LayerSwitcher/LayerSwitcher.spec.tsx index 68fcabb89d..8f7105f1fa 100644 --- a/src/LayerSwitcher/LayerSwitcher.spec.tsx +++ b/src/LayerSwitcher/LayerSwitcher.spec.tsx @@ -1,4 +1,5 @@ -import { render, screen,within } from '@testing-library/react'; +import { renderInMapContext } from '@terrestris/react-util/dist/Util/rtlTestUtils'; +import { screen, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import OlLayer from 'ol/layer/Layer'; import OlLayerTile from 'ol/layer/Tile'; @@ -37,24 +38,24 @@ describe('', () => { }); it('can be rendered', () => { - const { container } = render(); + const { container } = renderInMapContext(map, ); expect(container).toBeVisible(); }); it('contains map element', () => { - const { container } = render(); + const { container } = renderInMapContext(map, ); const mapElement = within(container).getByRole('menu'); expect(mapElement).toBeVisible(); }); it('adds a custom className', () => { - render(); + renderInMapContext(map, ); const firstChild = screen.getByRole('menu'); expect(firstChild).toHaveClass('peter'); }); it('passes style prop', () => { - render(); @@ -68,7 +69,7 @@ describe('', () => { }); it('sets all but one layer to invisible', () => { - render(); + renderInMapContext(map, ); const layer0visibile = layers[0].getVisible(); const layer1visibile = layers[1].getVisible(); expect(layer0visibile && layer1visibile).toBe(false); @@ -76,7 +77,7 @@ describe('', () => { }); it('switches the visible layer on click', async () => { - const { container } = render(); + const { container } = renderInMapContext(map, ); const switcher = within(container).getByRole('button'); const layer0visible = layers[0].getVisible(); @@ -90,7 +91,7 @@ describe('', () => { it('assumes the first provided layer as visible if the initial visibility of all layers is false', () => { layers.forEach(l => l.setVisible(false)); - render(); + renderInMapContext(map, ); expect(layers[0].getVisible()).toBeTruthy(); }); diff --git a/src/LayerSwitcher/LayerSwitcher.tsx b/src/LayerSwitcher/LayerSwitcher.tsx index 0030977c2a..44db22e38e 100644 --- a/src/LayerSwitcher/LayerSwitcher.tsx +++ b/src/LayerSwitcher/LayerSwitcher.tsx @@ -1,5 +1,6 @@ import './LayerSwitcher.less'; +import useMap from '@terrestris/react-util/dist/Hooks/useMap/useMap'; import OlLayerBase from 'ol/layer/Base'; import OlLayerGroup from 'ol/layer/Group'; import OlLayerTile from 'ol/layer/Tile'; @@ -24,10 +25,6 @@ export interface OwnProps { * The layers to be available in the switcher. */ layers: OlLayerBase[]; - /** - * The main map the layers should be synced with. - */ - map: OlMap; /** * The property that identifies the layer. */ @@ -47,11 +44,11 @@ export type LayerSwitcherProps = OwnProps & React.HTMLAttributes export const LayerSwitcher: React.FC = ({ identifierProperty = 'name', labelProperty = 'name', - map, layers, className: classNameProp, ...passThroughProps }) => { + const map = useMap(); const [switcherMap, setSwitcherMap] = useState(); @@ -67,19 +64,25 @@ export const LayerSwitcher: React.FC = ({ const className = `${CSS_PREFIX}layer-switcher`; /** - * Sets the visiblity of the layers in the props.map and this._map. + * Sets the visibility of the layers in the map and the switcherMap. * Also sets the previewLayer in the state. */ const updateLayerVisibility = useCallback(() => { - layers.forEach((l, i) => { - const visible = visibleLayerIndexRef.current === i; - l.setVisible(visible); - const clone = switcherMap?.getAllLayers()?.find(lc => lc.get(identifierProperty) === l.get(identifierProperty)); - if (clone) { - clone.setVisible(visible); - if (visible) { - setPreviewLayer(clone); - } + layers.forEach((layer, i) => { + layer.setVisible(visibleLayerIndexRef.current === i); + + const clone = switcherMap?.getAllLayers() + ?.find(lc => lc.get(identifierProperty) === layer.get(identifierProperty)); + + if (!clone) { + return; + } + + if ((visibleLayerIndexRef.current + 1) % layers.length === i) { + clone.setVisible(true); + setPreviewLayer(clone); + } else { + clone.setVisible(false); } }); }, [layers, identifierProperty, switcherMap]); @@ -123,6 +126,10 @@ export const LayerSwitcher: React.FC = ({ }, [switcherMap]); useEffect(() => { + if (!map) { + return; + } + const mapClone = new OlMap({ view: map.getView(), controls: [] @@ -146,15 +153,9 @@ export const LayerSwitcher: React.FC = ({ const onSwitcherClick = (evt: React.MouseEvent) => { evt.stopPropagation(); - layers.forEach((layer, index: number) => { - if (layer.getVisible()) { - if (layers.length - 1 === index) { - visibleLayerIndexRef.current = 0; - } else { - visibleLayerIndexRef.current = index + 1; - } - } - }); + const index = layers.findIndex(layer => layer.getVisible()); + + visibleLayerIndexRef.current = (index + 1) % layers.length; updateLayerVisibility(); }; From aeafdbad18cacd9a914bb842ef0b0a19e7400f4b Mon Sep 17 00:00:00 2001 From: Simon Seyock Date: Tue, 4 Jun 2024 17:52:37 +0200 Subject: [PATCH 2/2] test: use correct layerswitcher button to press --- src/LayerSwitcher/LayerSwitcher.spec.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/LayerSwitcher/LayerSwitcher.spec.tsx b/src/LayerSwitcher/LayerSwitcher.spec.tsx index 8f7105f1fa..7b2fac378d 100644 --- a/src/LayerSwitcher/LayerSwitcher.spec.tsx +++ b/src/LayerSwitcher/LayerSwitcher.spec.tsx @@ -78,7 +78,11 @@ describe('', () => { it('switches the visible layer on click', async () => { const { container } = renderInMapContext(map, ); - const switcher = within(container).getByRole('button'); + + // eslint-disable-next-line testing-library/no-container,testing-library/no-node-access + const layerSwitcher = container.querySelector('.react-geo-layer-switcher'); + + const switcher = within(layerSwitcher).getByRole('button'); const layer0visible = layers[0].getVisible(); const layer1visible = layers[1].getVisible();