Fix up latlngMode and improve readability of map-field-panel.js #52
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously the map initialisation was mostly in a function called "setAddress", but also had some in the `connect` method. We've split the Controller initialisation from the map initialisation, so it now hangs together a little better. Additionally, we now use controller properties in most places, rather than passing a bunch of arguments that are already accessible by the callee
haydngreatnews
force-pushed
the
fix/latlngmode
branch
from
118a573 to
20f3968
Compare
haydngreatnews
force-pushed
the
fix/latlngmode
branch
from
c73b5f1 to
8df771d
Compare
AndrewCalderSpringload
approved these changes
Jun 3, 2025
liamjohnston
approved these changes
Jun 4, 2025
liamjohnston
left a comment
liamjohnston
left a comment
There was a problem hiding this comment.
Changes seem good 👍
There's a few things we'd do differently if we were building this now/ourselves. Up to you if you want to tackle any of them though:
keyCodeis deprecated (see https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode)- we prefer to use
myEl.getAttribute('data-whatever')overmyEl.dataset.whatever, since you can search the code and find the references more easily. - typescript would be nice, but obvs that's a bit more work.
- ditto removing reliance on jquery (unless this also deals with jquery plugins?).
Collaborator
Author
|
Will make an issue recording those for now, so I'm not holding up our projects any longer than necessary |
Open
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.