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

Start refactoring scheme sketcher code, before moving to a library #512

Merged
merged 4 commits into from
Jul 1, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
43 changes: 15 additions & 28 deletions src/lib/draw/EditGeometryMode.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,10 @@
polygonTool,
routeTool,
} from "lib/draw/stores";
import type { FeatureWithAnyProps } from "lib/draw/types";
import { ButtonGroup, DefaultButton, SecondaryButton } from "govuk-svelte";
import type { FeatureWithProps } from "lib/maplibre";
import { interventionName } from "lib/sidebar/scheme_data";
import { schema } from "stores";
import { cfg } from "lib/draw/config";
import { onDestroy, onMount } from "svelte";
import type { FeatureUnion } from "types";
import PointControls from "./point/PointControls.svelte";
import PolygonControls from "./polygon/PolygonControls.svelte";
import RouteControls from "./route/RouteControls.svelte";
Expand All @@ -24,25 +22,23 @@
let name = "";
let controls = "";

// As a feature is being edited, store the latest version
let unsavedFeature: FeatureWithProps<Point | LineString | Polygon> | null =
null;
// As a feature is being edited, store the latest version. This type ignores
// the ID and specifics of the properties, but they're there.
let unsavedFeature: FeatureWithAnyProps | null = null;

onMount(() => {
let maybeFeature: FeatureUnion | null = null;
let maybeFeature: FeatureWithAnyProps | null = null;
gjSchemeCollection.update((gj) => {
maybeFeature = gj.features.find((f) => f.id == id)!;
// Hide it from the regular drawing while we're editing
maybeFeature.properties.hide_while_editing = true;
return gj;
});
let feature = maybeFeature!;
name = interventionName(feature);
name = cfg.interventionName(feature);

if (feature.geometry.type == "LineString") {
$routeTool?.editExistingRoute(
feature as unknown as Feature<LineString, RouteProps>,
);
$routeTool?.editExistingRoute(feature as Feature<LineString, RouteProps>);
$routeTool?.addEventListenerSuccess(onSuccess);
$routeTool?.addEventListenerUpdated(onUpdate);
$routeTool?.addEventListenerFailure(onFailure);
Expand Down Expand Up @@ -96,13 +92,14 @@
});
});

function onSuccess(feature: FeatureWithProps<Point | Polygon | LineString>) {
function onSuccess(feature: Feature<Point | Polygon | LineString>) {
feature.properties ??= {};
// Let onDestroy apply the update
unsavedFeature = feature;
unsavedFeature = feature as FeatureWithAnyProps;
mode.set({ mode: "edit-form", id });
}

function onUpdate(feature: FeatureWithProps<Polygon | LineString>) {
function onUpdate(feature: FeatureWithAnyProps<Polygon | LineString>) {
// Just remember the update; don't apply it yet
unsavedFeature = feature;
}
Expand All @@ -115,8 +112,8 @@

// Copy geometry and properties from source to destination
function updateFeature(
destination: FeatureUnion,
source: FeatureWithProps<Point | LineString | Polygon>,
destination: FeatureWithAnyProps,
source: FeatureWithAnyProps,
) {
destination.geometry = source.geometry;

Expand All @@ -129,17 +126,7 @@
if (source.properties.waypoints) {
destination.properties.waypoints = source.properties.waypoints;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The stuff staying here isn't domain specific; it comes from the route tool

// Only copy route_name if the user hasn't set it. It's not simple to
// distinguish the user manually editing the name from it being auto-filled
// previously, so be safe and don't overwrite anything. The user can always
// use the auto-fill button explicitly.
if (
source.properties.route_name &&
!destination.properties.name &&
$schema != "pipeline"
) {
destination.properties.name = source.properties.route_name;
}
cfg.updateFeature(destination, source);
}

function finish() {
Expand Down
9 changes: 5 additions & 4 deletions src/lib/draw/InterventionLayer.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
isPolygon,
layerId,
} from "lib/maplibre";
import { interventionName } from "lib/sidebar/scheme_data";
import { cfg } from "lib/draw/config";
import type {
DataDrivenPropertyValueSpecification,
ExpressionSpecification,
Expand All @@ -26,7 +26,8 @@
Popup,
type LayerClickInfo,
} from "svelte-maplibre";
import type { FeatureUnion, SchemeCollection } from "types";
import type { SchemeCollection } from "types";
import type { FeatureWithAnyProps } from "lib/draw/types";

$: gj = addLineStringEndpoints($gjSchemeCollection);

Expand Down Expand Up @@ -108,8 +109,8 @@

function tooltip(features: Feature[] | null): string {
if (features) {
let feature = features[0] as FeatureUnion;
let name = interventionName(feature);
let feature = features[0] as FeatureWithAnyProps;
let name = cfg.interventionName(feature);
let scheme =
$gjSchemeCollection.schemes[feature.properties.scheme_reference]
.scheme_name ?? "Untitled scheme";
Expand Down
83 changes: 83 additions & 0 deletions src/lib/draw/config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import type { FeatureUnion } from "types";
import {
newFeatureId,
getArbitrarySchemeRef,
gjSchemeCollection,
} from "lib/draw/stores";
import type { FeatureWithAnyProps } from "lib/draw/types";
import type { Feature, LineString, Polygon, Point } from "geojson";
import { interventionName } from "lib/sidebar/scheme_data";
import { schema } from "stores";
import { get } from "svelte/store";

// The draw code should be agnostic to the feature properties that differ by
// schema. Start centralizing the logic here, so it's easy for other users to
// override.

// TODO As an alternate idea, users could implement a custom Svelte store with methods for doing these things

export let cfg = {
interventionName: (f: FeatureWithAnyProps) => {
return interventionName(f as FeatureUnion);
},

newPointFeature: (f: Feature<Point>) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The theme here is that these functions get a "mostly just geometry" object and then make it obey the invariants imposed by gjSchemeCollection. Some of the logic is common,but stuff like intervention_type and fiddling with route names is not.

gjSchemeCollection.update((gj) => {
f.id = newFeatureId(gj);
f.properties ||= {};
f.properties.scheme_reference = getArbitrarySchemeRef(gj);
f.properties.intervention_type = "other";
// Typecast safe because we've established the invariants above
gj.features.push(f as FeatureUnion);
return gj;
});
},

newPolygonFeature: (f: Feature<Polygon>) => {
gjSchemeCollection.update((gj) => {
f.id = newFeatureId(gj);
f.properties ||= {};
f.properties.scheme_reference = getArbitrarySchemeRef(gj);
f.properties.intervention_type = "area";
f.properties.is_coverage_polygon = false;
// Typecast safe because we've established the invariants above
gj.features.push(f as FeatureUnion);
return gj;
});
},

newLineStringFeature: (f: Feature<LineString>) => {
gjSchemeCollection.update((gj) => {
f.id = newFeatureId(gj);
f.properties ||= {};
f.properties.scheme_reference = getArbitrarySchemeRef(gj);
f.properties.intervention_type = "route";
if (f.properties.route_name) {
if (get(schema) != "pipeline") {
f.properties.name = f.properties.route_name;
}
delete f.properties.route_name;
}
// Typecast safe because we've established the invariants above
gj.features.push(f as FeatureUnion);
return gj;
});
},

updateFeature: (
destination: FeatureWithAnyProps,
source: FeatureWithAnyProps,
) => {
// Only copy route_name if the user hasn't set it. It's not simple to
// distinguish the user manually editing the name from it being auto-filled
// previously, so be safe and don't overwrite anything. The user can always
// use the auto-fill button explicitly.
if (
source.properties.route_name &&
!destination.properties.name &&
get(schema) != "pipeline"
) {
destination.properties.name = source.properties.route_name;
}
},
};
25 changes: 5 additions & 20 deletions src/lib/draw/point/PointMode.svelte
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
<script lang="ts">
import type { Point } from "geojson";
import {
gjSchemeCollection,
mode,
newFeatureId,
pointTool,
} from "lib/draw/stores";
import type { Feature, Point } from "geojson";
import { mode, pointTool } from "lib/draw/stores";
import { SecondaryButton } from "govuk-svelte";
import type { FeatureWithProps } from "lib/maplibre";
import { getArbitraryScheme } from "lib/sidebar/scheme_data";
import { onDestroy, onMount } from "svelte";
import type { Feature } from "types";
import PointControls from "./PointControls.svelte";
import { cfg } from "lib/draw/config";

onMount(() => {
$pointTool!.start();
Expand All @@ -23,16 +16,8 @@
$pointTool!.clearEventListeners();
});

function onSuccess(feature: FeatureWithProps<Point>) {
gjSchemeCollection.update((gj) => {
feature.id = newFeatureId(gj);
feature.properties.scheme_reference =
getArbitraryScheme(gj).scheme_reference;
feature.properties.intervention_type = "other";
gj.features.push(feature as Feature<Point>);
return gj;
});

function onSuccess(feature: Feature<Point>) {
cfg.newPointFeature(feature);
mode.set({ mode: "edit-form", id: feature.id as number });
}

Expand Down
20 changes: 13 additions & 7 deletions src/lib/draw/point/point_tool.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,24 @@
import type { Point } from "geojson";
import { pointFeature, type FeatureWithProps } from "lib/maplibre";
import type { Feature, Point } from "geojson";
import { setPrecision } from "lib/draw/stores";
import type { Map, MapMouseEvent } from "maplibre-gl";

// Note this uses the geojson FeatureWithProps, not our specialization in types.ts
export class PointTool {
map: Map;
active: boolean;
eventListenersSuccess: ((f: FeatureWithProps<Point>) => void)[];
eventListenersSuccess: ((f: Feature<Point>) => void)[];
eventListenersFailure: (() => void)[];
cursor: FeatureWithProps<Point> | null;
cursor: Feature<Point> | null;

onMouseMove = (e: MapMouseEvent) => {
if (this.active) {
this.cursor = pointFeature(e.lngLat.toArray());
this.cursor = {
type: "Feature",
properties: {},
geometry: {
type: "Point",
coordinates: setPrecision(e.lngLat.toArray()),
},
};
}
};

Expand Down Expand Up @@ -62,7 +68,7 @@ export class PointTool {
this.stop();
}

addEventListenerSuccess(callback: (f: FeatureWithProps<Point>) => void) {
addEventListenerSuccess(callback: (f: Feature<Point>) => void) {
this.eventListenersSuccess.push(callback);
}
addEventListenerFailure(callback: () => void) {
Expand Down
26 changes: 5 additions & 21 deletions src/lib/draw/polygon/PolygonMode.svelte
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
<script lang="ts">
import type { Polygon } from "geojson";
import {
gjSchemeCollection,
mode,
newFeatureId,
polygonTool,
} from "lib/draw/stores";
import type { Feature, Polygon } from "geojson";
import { mode, polygonTool } from "lib/draw/stores";
import { ButtonGroup, DefaultButton, SecondaryButton } from "govuk-svelte";
import type { FeatureWithProps } from "lib/maplibre";
import { getArbitraryScheme } from "lib/sidebar/scheme_data";
import { onDestroy, onMount } from "svelte";
import type { Feature } from "types";
import PolygonControls from "./PolygonControls.svelte";
import { cfg } from "lib/draw/config";

onMount(() => {
$polygonTool!.startNew();
Expand All @@ -23,17 +16,8 @@
$polygonTool!.clearEventListeners();
});

function onSuccess(feature: FeatureWithProps<Polygon>) {
gjSchemeCollection.update((gj) => {
feature.id = newFeatureId(gj);
feature.properties.scheme_reference =
getArbitraryScheme(gj).scheme_reference;
feature.properties.intervention_type = "area";
feature.properties.is_coverage_polygon = false;
gj.features.push(feature as Feature<Polygon>);
return gj;
});

function onSuccess(feature: Feature<Polygon>) {
cfg.newPolygonFeature(feature);
mode.set({ mode: "edit-form", id: feature.id as number });
}

Expand Down
33 changes: 6 additions & 27 deletions src/lib/draw/route/RouteMode.svelte
Original file line number Diff line number Diff line change
@@ -1,18 +1,10 @@
<script lang="ts">
import type { LineString, Polygon } from "geojson";
import {
gjSchemeCollection,
mode,
newFeatureId,
routeTool,
} from "lib/draw/stores";
import type { Feature, LineString, Polygon } from "geojson";
import { mode, routeTool } from "lib/draw/stores";
import { ButtonGroup, DefaultButton, SecondaryButton } from "govuk-svelte";
import type { FeatureWithProps } from "lib/maplibre";
import { getArbitraryScheme } from "lib/sidebar/scheme_data";
import { schema } from "stores";
import { onDestroy, onMount } from "svelte";
import type { Feature } from "types";
import RouteControls from "./RouteControls.svelte";
import { cfg } from "lib/draw/config";

onMount(() => {
$routeTool!.startRoute();
Expand All @@ -24,22 +16,9 @@
$routeTool!.clearEventListeners();
});

function onSuccess(feature: FeatureWithProps<LineString | Polygon>) {
gjSchemeCollection.update((gj) => {
feature.id = newFeatureId(gj);
feature.properties.scheme_reference =
getArbitraryScheme(gj).scheme_reference;
feature.properties.intervention_type = "route";
if (feature.properties.route_name) {
if ($schema != "pipeline") {
feature.properties.name = feature.properties.route_name;
}
delete feature.properties.route_name;
}
gj.features.push(feature as Feature<LineString>);
return gj;
});

function onSuccess(feature: Feature<LineString | Polygon>) {
// We did startRoute, so we know it's a LineString
cfg.newLineStringFeature(feature as Feature<LineString>);
mode.set({ mode: "edit-form", id: feature.id as number });
}

Expand Down
Loading
Loading