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

188071551 locations setup #15

Merged
merged 10 commits into from
Aug 8, 2024
Merged

188071551 locations setup #15

merged 10 commits into from
Aug 8, 2024

Conversation

bacalj
Copy link
Collaborator

@bacalj bacalj commented Aug 8, 2024

This adds the maintenance of an array of locations.

Locations are potentially updated on three events:

  1. Get data button
  2. Clear data button
  3. CODAP-side deletion of cases

Locations are used to

  1. check if we already have data for a given location
  2. in an upcoming feature, populate a list of existing locations that can be selected to move lat/long in sim

@bacalj bacalj requested a review from pjanik August 8, 2024 11:38
Copy link
Member

@pjanik pjanik left a comment

Choose a reason for hiding this comment

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

Looks great, thanks 👍

name: c.values.location,
latitude: c.values.latitude,
longitude: c.values.longitude,
coordinatePair: `(${c.values.latitude},${c.values.longitude})`
Copy link
Member

Choose a reason for hiding this comment

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

Is the idea here to have something like a unique ID/key that can be easily compared? It's a pretty cool idea. However, I think I'll need a helper function like locationsEqual = (a, b) => a.lat === b.lat && a.long === b.long to check if a given lat/long matches any option in the dropdown. Then we could probably skip storing this derived property. This equality helper should probably be universal throughout the whole app. Alternatively, another option could be to generate this ID/key from the lat/long pair and compare the result. But it's pretty minor dilemma. 😉

@pjanik pjanik merged commit e6dca32 into main Aug 8, 2024
5 checks passed
@pjanik pjanik deleted the 188071551-user-locations-setup branch August 8, 2024 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants