Editing

[m-elseberg]

Editing capabilities are, of course, a central part of many GIS apps. However, the current implementation is limited to polygons and the OGC Features API.

This pull request suggests a more general-purpose editing solution, which evolved throughout one of our projects. Features include:

• creating and updating any geometry type (polygons, line strings, points, circles),
• pre-configurable or fully custom attribute forms,
• ability to save features anywhere,
• snapping.

To test this implementation, open the following local sample: http://localhost:5173/samples/editing-sample/. For ease of testing, this sample saves the created features to the browser's IndexedDB.

This solution is not expected to be feature-complete or to meet everyone's needs. For one thing, it is still lacking documentation.

(screenshots: 1, 2)

[changeset-bot]

⚠️ No Changeset found

Latest commit: 23005b3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[mbeckem]

I updated the vite config and license report config to get the samples to build. Nitpick: the react-color dependency looks unmaintained and references some very old packages (e.g. @material/icons). Should probably use an alternative.

[github-actions]

PR Preview Action v1.4.7
🚀 Deployed preview to https://open-pioneer.github.io/trails-pr-previews/openlayers-base-packages/pr-previews/pr-387/
on branch gh-pages at 2025-02-20 14:11 UTC

[mbeckem]

Okay, i've finally had a chance to look at your PR. First of all, the end user experience seems to be (almost) ideal, great job.

Looking at the implementation, i've got some feedback to share with you.
Not everything is extremely important, but some of it is: I feel that it is important to start out with a high quality implementation (also maintenance-vise) because this package will probably be quite heavily used.

• Tests are missing missing.

• Really needs comments, at least on all exported entities and their properties.

• Are all exports necessary? I think the public interface could be slimmed down a bit without losing functionality.

• Ensure nice behavior with dynamic size (no fixed size or useless scrollbars)

• Icon Buttons should probably have a tooltip (could be same as aria label).

• Eliminate FieldInputsProvider and create a FormContent concept (FieldInput[] or completely custom Form). This can also replace the children API.

• ColorInput is difficult to support well and should be removed for the time being.

  • Note that chakra v3 has a color input widget so it might be possible to re-add this feature at a later time.

• Idea: CustomInput type that supports free form rendering, i.e. it renders some react component within the form.
  This way, arbitrary input types can be added by developers (including ColorInput).
  Requires a render function (might have to pass some props in for aria stuff, form ids etc.).

• Union variants should be exported if the unions are exported as well. For example InitialStep as a developer might want to name the type in their code.

• For consistency with other packages:

  • If raw OpenLayers layers appear in APIs, use olLayer instead of layer to avoid confusion
  • Or (even better, if possible), use the Layer type (or other fitting types) from the map package instead

• I like the different *Handler functions.
  Their signatures would have to be changed in the future if we ever get around creating our own Feature class.
  Currently, we have BaseFeature in the map package (a barebones interface that combines attributes with geometry and projection)
  but we would like to create a fully featured Feature class at some point (as a reactive Model, possibly using OL's Feature internally).

• PropertyFormContext might be a performance problem in the future.
  In react, changing the context value requires rerendering all children that use that context.
  Combined with react-style immutability this can be a performance hazard.
  The reactivity API could be used to make a FormModel available to all children, that would not this problem (because of fine grained mutability, the model would stay the same but small details would change).

• Not sure why FeatureTemplate can be considered an Action, this feels wrong.
  Better: {kind: "create", template: FeatureTemplate} ?
  This also allows for easy future extensions with more action types.

• Not a fan of the tsx directory, I think that directory level can be removed.

• Please don't use console.* for logging, use the Logger type from @open-pioneer/core instead.
  You can simplify create a module level constant, like const LOG = createLogger("map:AbstractLayer");.

• I think using the reactivity API for the editing actions and the undo management might be easier than the current approach.

• Nitpick: I think these type aliases hurt readability more than they help.

  export type Callback<T> = (newValue: T) => void;
  export type VoidCallback = () => void;
  export type AsyncVoidCallback = () => Promise<void>;

• Nitpick: camel case for input types (e.g. textArea) feels wrong (-> "text-area" instead)

• Its nice that this works but it's pretty risky to rely on it.
  The this parameter has not been bound explicitly (by us), although it might be a feature of FormatJS:

  const { formatMessage } = useIntl();
  

• Just a tip: take a look at useEvent, it can help simplifying some usages of useCallback (no need to track dependencies).