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

[Basemap-switcher] Use a image based basemap switcher #362

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/witty-lemons-look.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@open-pioneer/basemap-switcher": patch
---

Add optional image basemap switcher
177 changes: 109 additions & 68 deletions pnpm-lock.yaml

Large diffs are not rendered by default.

105 changes: 105 additions & 0 deletions src/packages/imageBasemap-switcher/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
# @open-pioneer/basemap-switcher

## 0.4.2

### Patch Changes

- 4140646: Update trails dependencies
- 4140646: Update to react 18.3.1
- 81bc7da: Update trails dependencies
- 2c092dc: Update dependencies
- Updated dependencies [4140646]
- Updated dependencies [4140646]
- Updated dependencies [b5bb7a1]
- Updated dependencies [81bc7da]
- Updated dependencies [2c092dc]
- Updated dependencies [4140646]
- @open-pioneer/[email protected]
- @open-pioneer/[email protected]

## 0.4.1

### Patch Changes

- Updated dependencies [520a97b]
- @open-pioneer/[email protected]

## 0.4.0

### Minor Changes

- 9334e81: Update to OpenLayers 9

### Patch Changes

- 1a8ad89: Update package.json metadata
- 6162979: Update versions of core packages
- Updated dependencies [1a8ad89]
- Updated dependencies [a11bf72]
- Updated dependencies [fc6bf82]
- Updated dependencies [a0d8882]
- Updated dependencies [6162979]
- Updated dependencies [9334e81]
- Updated dependencies [ac7fdd1]
- Updated dependencies [13ea342]
- @open-pioneer/[email protected]
- @open-pioneer/[email protected]

## 0.3.1

### Patch Changes

- Updated dependencies [611ddb9]
- @open-pioneer/[email protected]

## 0.3.0

### Minor Changes

- ee7c2d4: Update runtime version.

### Patch Changes

- 6984d20: Adjusted basemap switcher style (changed background color of select indicator and cursor style).
- 0883bbd: Add property `menuPosition` to React Select.
- Updated dependencies [ee7c2d4]
- Updated dependencies [a582e5e]
- Updated dependencies [0456500]
- Updated dependencies [762e7b9]
- @open-pioneer/[email protected]
- @open-pioneer/[email protected]

## 0.2.0

### Minor Changes

- 70349a8: Update to new core packages major versions

### Patch Changes

- Updated dependencies [70349a8]
- @open-pioneer/[email protected]
- @open-pioneer/[email protected]

## 0.1.1

### Patch Changes

- Updated dependencies [08bffbc]
- Updated dependencies [a58546b]
- Updated dependencies [a58546b]
- Updated dependencies [0c4ce04]
- @open-pioneer/[email protected]

## 0.1.0

### Minor Changes

- 1e7545c: Initial release.

### Patch Changes

- Updated dependencies [bb2f27a]
- Updated dependencies [182da1c]
- @open-pioneer/[email protected]
- @open-pioneer/[email protected]
216 changes: 216 additions & 0 deletions src/packages/imageBasemap-switcher/ImageBasemapSwitcher.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
// SPDX-FileCopyrightText: 2023 Open Pioneer project (https://github.com/open-pioneer)
// SPDX-License-Identifier: Apache-2.0
import {
Box,
Button,
Flex,
Menu,
MenuButton,
MenuList,
Tooltip
} from "@open-pioneer/chakra-integration";
import { Image, MenuItem } from "@chakra-ui/react";
import { Layer, MapModel, useMapModel } from "@open-pioneer/map";
import { useIntl } from "open-pioneer:react-hooks";
import { FC, useCallback, useMemo, useRef, useSyncExternalStore } from "react";
import { CommonComponentProps, useCommonComponentProps } from "@open-pioneer/react-utils";

import defaultImage from "./assets/map_699730.png"; // https://de.freepik.com/icon/karte_699730#fromView=search&page=1&position=9&uuid=affc743f-cf4c-4469-8919-f587e95295db
import emptyImage from "./assets/cancel_4968337.png"; // https://de.freepik.com/icon/stornieren_4968337#fromView=search&page=1&position=13&uuid=8105ed6b-5f5a-41e2-b0df-392a9013a711

const NO_BASEMAP_ID = "___NO_BASEMAP___";

export interface ImageLabelSwitchObject {
image: string;
label: string;
callBack: () => void;
}

export interface BaselayerImageBasemapSwitcherProps {
image: string;
label: string;
}

/**
* These are special properties for the BasemapSwitcher.
*/
export interface ImageBasemapSwitcherProps extends CommonComponentProps {
/**
* The id of the map.
*/
mapId: string;

/**
* Additional css class name(s) that will be added to the BasemapSwitcher component.
*/
className?: string;

/**
* Specifies whether an option to deactivate all basemap layers is available in the BasemapSwitcher.
* Defaults to `false`.
*/
allowSelectingEmptyBasemap?: boolean | undefined;

/**
* Optional map of images and labels to the specific basemaps layers.
* Alternatively, the image and label can be set in the layer attributes.
* If no images are set, the default image will be used.
*/
imageMap?: Map<string, BaselayerImageBasemapSwitcherProps>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using Map<> in React is not ideal:
Maps are mutable, if someone updates the Map (set, delete) from the outside this will not trigger a re-render of the component.
I suggest using a immutable type like Record, Array or a custom (immutable) object.


/**
* Optional filter to exclude basemaps from the switcher.
*/
excludeBasemapWithIdFilter?: string[];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose using a filter function instead of a list of IDs.
This would allow greater flexibility.

}

/**
* The `BasemapSwitcher` component can be used in an app to switch between the different basemaps.
*/
export const ImageBasemapSwitcher: FC<ImageBasemapSwitcherProps> = (props) => {
const intl = useIntl();
const {
mapId,
allowSelectingEmptyBasemap = false,
imageMap,
excludeBasemapWithIdFilter
} = props;
const { containerProps } = useCommonComponentProps("image-basemap-switcher", props);
const emptyBasemapLabel = intl.formatMessage({ id: "emptyBasemapLabel" });

const { map } = useMapModel(mapId);
const baseLayers = useBaseLayers(map);

const setImageLabelObject = useCallback(
(selectedLayer: Layer | undefined) => {
const activateLayer = (layerId: string) => {
map?.layers.activateBaseLayer(layerId === NO_BASEMAP_ID ? undefined : layerId);
};

const basemap = selectedLayer?.attributes
?.imageBasemapSwitcher as BaselayerImageBasemapSwitcherProps;
const parameter = imageMap?.get(selectedLayer?.id || NO_BASEMAP_ID);
const image =
basemap?.image || parameter?.image || (!selectedLayer ? emptyImage : defaultImage);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be more intuitive if parameter.image has precedence.
Typically (e.g. with CSS), properties directly on the component have precedents over definitions elsewhere.

const label =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regarding precedence see comment above

basemap?.label ||
parameter?.label ||
selectedLayer?.title ||
(!selectedLayer ? emptyBasemapLabel : "basemap");

return {
image: image,
label: label,
callBack: () => activateLayer(selectedLayer?.id || NO_BASEMAP_ID)
};
},
[emptyBasemapLabel, imageMap, map?.layers]
);

const { selectedImageLabel, choosableImageLabel } = useMemo(() => {
const activeBaseLayer = map?.layers.getActiveBaseLayer();
const selectedLayer = baseLayers.find((l) => l === activeBaseLayer);

const selectedImageLabel = setImageLabelObject(selectedLayer);

const choosableImageLabel = [] as ImageLabelSwitchObject[];
baseLayers.forEach((layer) => {
if (
excludeBasemapWithIdFilter &&
!excludeBasemapWithIdFilter.includes(layer.id || "")
) {
return;
}
choosableImageLabel.push(setImageLabelObject(layer));
});
if (allowSelectingEmptyBasemap) {
choosableImageLabel.push(setImageLabelObject(undefined));
}

return { selectedImageLabel, choosableImageLabel };
}, [
map?.layers,
baseLayers,
setImageLabelObject,
allowSelectingEmptyBasemap,
excludeBasemapWithIdFilter
]);
Comment on lines +110 to +137
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment at useBaseLayers


return (
<Box {...containerProps}>
{
<Menu>
<Flex>
<Tooltip label={selectedImageLabel?.label}>
<MenuButton as={Button} className={"image-basemap-front-image"}>
<Image width={"40px"} src={selectedImageLabel?.image}></Image>
</MenuButton>
</Tooltip>
</Flex>
<MenuList display={"contents"} overflowY={"auto"}>
{choosableImageLabel.map((imageLabel, index) => {
return (
<BasemapOnMapSwitcherElement
key={imageLabel.label + index}
src={imageLabel.image}
label={imageLabel.label}
callback={imageLabel.callBack}
/>
);
})}
</MenuList>
</Menu>
}
</Box>
);
};

function useBaseLayers(mapModel: MapModel | undefined): Layer[] {
// Caches potentially expensive layers arrays.
// Not sure if this is a good idea, but getSnapshot() should always be fast.
// If this is a no-go, make getAllLayers() fast instead.
const baseLayers = useRef<Layer[] | undefined>();
const subscribe = useCallback(
(cb: () => void) => {
// Reset cache when (re-) subscribing
baseLayers.current = undefined;

if (!mapModel) {
return () => undefined;
}
const resource = mapModel.layers.on("changed", () => {
// Reset cache content so getSnapshot() fetches basemaps again.
baseLayers.current = undefined;
cb();
});
return () => resource.destroy();
},
[mapModel]
);
const getSnapshot = useCallback(() => {
if (baseLayers.current) {
return baseLayers.current;
}
return (baseLayers.current = mapModel?.layers.getBaseLayers() ?? []);
}, [mapModel]);
return useSyncExternalStore(subscribe, getSnapshot);
}
Comment on lines +168 to +197
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could hugely benefit from using the Reactivty API (see Comment).
If I am not mistake, caching current base layers and filtering + memoizing image labels could be packed into a single use of useReactiveSnapshot.


interface BasemapOnMapSwitcherElementProps {
src: string;
label: string;
callback: () => void;
width?: string;
height?: string;
}

export function BasemapOnMapSwitcherElement(props: BasemapOnMapSwitcherElementProps) {
const { src, label, callback, width, height } = props;
return (
<MenuItem onClick={() => callback()}>
<Tooltip label={label}>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thumbs up if aria-labels are supported

<Image src={src} width={width || "60px"} height={height || "40px"}></Image>
</Tooltip>
</MenuItem>
);
}
Loading