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

Crossings layer for browse page #343

Merged
merged 6 commits into from
Sep 18, 2023
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
2 changes: 2 additions & 0 deletions src/lib/browse/LayerControls.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import CensusOutputAreaLayerControl from "./layers/CensusOutputAreaLayerControl.svelte";
import CombinedAuthoritiesLayerControl from "./layers/CombinedAuthoritiesLayerControl.svelte";
import CriticalIssuesLayerControl from "./layers/CriticalIssuesLayerControl.svelte";
import CrossingsLayerControl from "./layers/CrossingsLayerControl.svelte";
import CycleParkingLayerControl from "./layers/CycleParkingLayerControl.svelte";
import CyclePathsLayerControl from "./layers/CyclePathsLayerControl.svelte";
import HospitalsLayerControl from "./layers/HospitalsLayerControl.svelte";
Expand Down Expand Up @@ -52,6 +53,7 @@
<MrnLayerControl />
<BusRoutesLayerControl />
<CycleParkingLayerControl />
<CrossingsLayerControl />
</CheckboxGroup>
</CollapsibleCard>
<CollapsibleCard label="Boundaries">
Expand Down
1 change: 1 addition & 0 deletions src/lib/browse/colors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const colors = {
bus_route_with_lane: "#9362BA",
bus_route_without_lane: "#C2A6D8",
cycle_parking: "black",
crossings: "green",

// Thanks to https://github.com/cyipt/cyipt-website/issues/23
cycle_paths: {
Expand Down
93 changes: 93 additions & 0 deletions src/lib/browse/layers/CrossingsLayerControl.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
<script lang="ts">
import {
ColorLegend,
ExternalLink,
HelpButton,
InteractiveLayer,
} from "lib/common";
import { Checkbox } from "lib/govuk";
import { overwriteCircleLayer, overwritePmtilesSource } from "lib/maplibre";
import type { MapGeoJSONFeature } from "maplibre-gl";
import { map } from "stores";
import { colors } from "../colors";

const name = "crossings";
let show = false;

function tooltip(feature: MapGeoJSONFeature): string {
const crossingTypeString = feature.properties.crossing;
if (crossingTypeString === "no")
Copy link
Contributor

Choose a reason for hiding this comment

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

Does npm run fmt keep this? Lack of braces for an if are weird

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this special and not in the mapping below? (and in case my other comment got lost, the style of using if without braces is different than everywhere else in the codebase. prettier doesn't change it right now, so just be aware it introduces inconsistency

return "Location where crossing is impossible/illegal but where there is a clear desire line to cross.";
let result = "Crossing with the following features.";
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks out-of-date and almost never used. I think you can just change all of this code to do descriptions.get(crossingType) ?? crossingType

const initialTooltipSentence =
initialTooltipSentences.get(crossingTypeString);
if (initialTooltipSentence) result = initialTooltipSentence;

return `<p>${result}. Click for details.</p>`;
}

const initialTooltipSentences: Map<string, string> = new Map<string, string>([
Copy link
Contributor

Choose a reason for hiding this comment

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

In general in TS, I don't think you need to do let x: Type = new Foo<Generic>(... -- probably just the first is sufficient.

["traffic_signals", "Signalised crossing"],
["marked", "Crossing with no traffic signals"],
["uncontrolled", "Crossing with no traffic signals"],
["unmarked", "Crossing with no markings or signals"],
["zebra", "Zebra crossing"],
["island", "Crossing with an island"],
[
"informal",
"Informal crossing with an obvious desire line, but no official infrastructure to support it",
],
]);

overwritePmtilesSource(
$map,
name,
`${import.meta.env.VITE_RESOURCE_BASE}/layers/v1/${name}.pmtiles`
);
let color = colors[name];

overwriteCircleLayer($map, {
id: name,
source: name,
sourceLayer: name,
color: color,
radius: ["interpolate", ["linear"], ["zoom"], 1, 2, 8, 3, 13, 10],
});

function onClick(e: CustomEvent<MapGeoJSONFeature>) {
window.open(
`http://openstreetmap.org/node/${e.detail.properties.osm_id}`,
"_blank"
);
}
</script>

<Checkbox id={name} bind:checked={show}>
<ColorLegend {color} />
Crossings
<span slot="right">
<HelpButton>
<p>
This shows <ExternalLink
href="https://wiki.openstreetmap.org/wiki/Key:crossing"
>
crossing
</ExternalLink> data from OpenStreetMap (as of 9 August 2023).
</p>

<p>
License: <ExternalLink href="https://www.openstreetmap.org/copyright">
Open Data Commons Open Database License
</ExternalLink>
</p>
</HelpButton>
</span>
</Checkbox>

<InteractiveLayer
layer={name}
{tooltip}
{show}
clickable={true}
on:click={onClick}
/>
56 changes: 0 additions & 56 deletions src/lib/browse/layers/PointAmenityLayerControl.svelte

This file was deleted.

78 changes: 57 additions & 21 deletions src/lib/browse/layers/RailwayStationsLayerControl.svelte
Original file line number Diff line number Diff line change
@@ -1,28 +1,64 @@
<script lang="ts">
import { circleRadius } from "colors";
import { ExternalLink } from "lib/common";
import PointAmenityLayerControl from "./PointAmenityLayerControl.svelte";
import {
ColorLegend,
ExternalLink,
HelpButton,
InteractiveLayer,
} from "lib/common";
import { Checkbox } from "lib/govuk";
import { overwriteCircleLayer, overwriteSource } from "lib/maplibre";
import type { MapGeoJSONFeature } from "maplibre-gl";
import { map } from "stores";
import { colors } from "../colors";

const name = "railway_stations";
const url = `${import.meta.env.VITE_RESOURCE_BASE}/layers/v1/${name}.geojson`;
let show = false;

overwriteSource($map, name, url);
let color = colors[name];

overwriteCircleLayer($map, {
id: name,
source: name,
color: color,
radius: circleRadius / 2,
// TODO Outline?
});

function tooltip(feature: MapGeoJSONFeature): string {
let name = feature.properties.name ?? "Unnamed railway station";
return `<p>${name}</p>`;
}
</script>

<PointAmenityLayerControl
{name}
singularNoun="railway station"
pluralNoun="Railway stations"
{circleRadius}
>
<p>
This shows <ExternalLink
href="https://wiki.openstreetmap.org/wiki/Tag:railway%3Dstation"
>
railway station
</ExternalLink> data from OpenStreetMap (as of 9 August 2023).
</p>
<Checkbox id={name} bind:checked={show}>
<ColorLegend {color} />
Railway Stations
<span slot="right">
<HelpButton>
<p>
This shows <ExternalLink
href="https://wiki.openstreetmap.org/wiki/Tag:railway%3Dstation"
>
railway station
</ExternalLink> data from OpenStreetMap (as of 9 August 2023).
</p>

<p>
License: <ExternalLink href="https://www.openstreetmap.org/copyright">
Open Data Commons Open Database License
</ExternalLink>
</p>
</HelpButton>
</span>
</Checkbox>

<p>
License: <ExternalLink href="https://www.openstreetmap.org/copyright">
Open Data Commons Open Database License
</ExternalLink>
</p>
</PointAmenityLayerControl>
<InteractiveLayer
layer={name}
{tooltip}
{show}
clickable={false}
on:click={() => {}}
Copy link
Contributor

Choose a reason for hiding this comment

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

You can remove this

/>
1 change: 1 addition & 0 deletions src/lib/maplibre/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ const layerZorder = [
"bus_routes",
"railway_stations",
"cycle_parking",
"crossings",
"cycle_paths",
"vehicle_counts",

Expand Down
Loading