-
Notifications
You must be signed in to change notification settings - Fork 4
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 a page for critical issue entry #330
Conversation
@@ -37,6 +37,8 @@ | |||
isInactive = false; | |||
$map.on("click", handleMapClickEvent); | |||
$map.getCanvas().style.cursor = `url(${cameraCursorUrl}), auto`; | |||
// Create these as late as possible, so changes to basemap layers are used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The bug otherwise is to switch baselayers, then turn on stview. The layers from dataviz would be used.
"21 - Routes must join other facilities as a network", | ||
]); | ||
|
||
function listToChoices(list: string[]): [string, string][] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we have a DB and can be opinionated about how we store things, I'd love to use a real enum in many places, or at least the codes defined in the Lookup sheet. But for now, no point; we need to exactly match the existing xlsx
} | ||
|
||
function exportForm() { | ||
// Trivial validation currently |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about moving the validation responsibility to each component (with something like a required
bool for select, text input, etc... or a custom validation bit). But https://design-system.service.gov.uk/patterns/validation/ recommends not showing errors until the user tries to submit, since the form would visually change as people tab between fields. So for now, doing something simple.
function exportForm() { | ||
// Trivial validation currently | ||
inspectorError = inspector ? "" : "Required"; | ||
schemeReferenceError = schemeReference ? "" : "Required"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One of the big asks was to validate this field. I talked to Bec, and the current recommendation isn't to do anything. We should be using scheme IDs, not references, but that'll take a bigger change to the xlsx and current workflow. If we really want to do something else in the meantime, we can just copy ~1300 valid scheme references here and use typeahead / verify the input's in the set.
return; | ||
} | ||
// TODO Don't overwrite something the user entered | ||
let url = `https://nominatim.openstreetmap.org/reverse?lat=${pt.lat}&lon=${pt.lng}&format=json`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See https://nominatim.org/release-docs/latest/api/Reverse/#examples for reference. Fine for our super-low QPS usage, and the results seem fine to me.
$map.getCanvas().style.cursor = "inherit"; | ||
markerPosition = marker!.getLngLat(); | ||
}); | ||
// TODO Change cursor when we're hovering on the marker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Markers are actually DOM elements on top of the webgl canvas AFAICT, so I think mouseenter` will work, but haven't fiddled around yet. Couldn't find examples online, oddly. https://maplibre.org/maplibre-gl-js/docs/examples/drag-a-marker/
src/pages/BrowseSchemes.svelte
Outdated
@@ -1,7 +1,6 @@ | |||
<script lang="ts"> | |||
// @ts-ignore no declarations | |||
import { initAll } from "govuk-frontend"; | |||
import "../style/main.css"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noticed this was imported twice here
}); | ||
|
||
let markerPosition: LngLat | null = null; | ||
let streetviewOff = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bit of odd mutual exclusion logic here. If stview is on, clicking shouldn't create a new pin. And while stview is on, you can't switch basemap, because the road helper stuff would then need to detect that change and fiddle with the correct layers.
796147d
to
9141d64
Compare
How do we feel about this? The future tasks are meant for another PR, pending feedback |
48f8ba6
to
a19e51e
Compare
Rebased, and fixed a bug Pete found where the contents of the textbox in the popup didn't update (but the clipboard did) |
Demo at https://acteng.github.io/atip/critical_entry_mvp/critical_entry.html. A start to #322:
screencast.mp4
Pasting the resulting row in the xlsx appears to work for me, but of course we should test more carefully end-to-end before recommending people use this.
Future steps: