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

Basic AoIs #690

Merged
merged 10 commits into from
Oct 12, 2023
Merged

Basic AoIs #690

merged 10 commits into from
Oct 12, 2023

Conversation

nerik
Copy link
Contributor

@nerik nerik commented Oct 4, 2023

Prototypes a simple AoI control working with the new map. This includes storing the AoI(s) in the URL.

@netlify
Copy link

netlify bot commented Oct 4, 2023

Deploy Preview for veda-ui ready!

Name Link
🔨 Latest commit 2b89c94
🔍 Latest deploy log https://app.netlify.com/sites/veda-ui/deploys/65266e41f4cd150008b02ed8
😎 Deploy Preview https://deploy-preview-690--veda-ui.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@nerik nerik force-pushed the feature/exploration-aoi branch from 5c227a7 to 530acb1 Compare October 5, 2023 08:50
@nerik nerik marked this pull request as ready for review October 6, 2023 13:45
@nerik nerik force-pushed the feature/exploration-aoi branch from 3221d5b to 781701b Compare October 6, 2023 13:57
@nerik nerik force-pushed the feature/exploration-aoi branch from 781701b to 9bca0b4 Compare October 6, 2023 18:51
Copy link
Collaborator

@danielfdsilva danielfdsilva left a comment

Choose a reason for hiding this comment

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

A couple of things I'd suggest maybe adding/changing on this PR:

  • the cursor over the map when the drawing is active
  • disabling the trash can when nothing can be deleted
  • jotai location uses hash to store the values. Query parameters are better suited for this purpose - can we use that instead?

app/scripts/components/exploration/atoms/atoms.ts Outdated Show resolved Hide resolved
Comment on lines 62 to 101
export const aoisUpdateGeometryAtom = atom(
null,
(get, set, updates: { id: string; geometry: Polygon }[]) => {
let newFeatures = [...get(aoisFeaturesAtom)];
updates.forEach(({ id, geometry }) => {
const existingFeature = newFeatures.find((feature) => feature.id === id);
if (existingFeature) {
existingFeature.geometry = geometry;
} else {
const newFeature: AoIFeature = {
type: 'Feature',
id,
geometry,
selected: true,
properties: {}
};
newFeatures = [...newFeatures, newFeature];
}
});
set(aoisHashAtom, encodeAois(newFeatures));
}
);

// Setter atom to update AOIs selected state, writing directly to the hash atom.
export const aoisSetSelectedAtom = atom(null, (get, set, ids: string[]) => {
const features = get(aoisFeaturesAtom);
const newFeatures = features.map((feature) => {
return { ...feature, selected: ids.includes(feature.id as string) };
});
set(aoisHashAtom, encodeAois(newFeatures));
});

// Setter atom to delete AOIs, writing directly to the hash atom.
export const aoisDeleteAtom = atom(null, (get, set, ids: string[]) => {
const features = get(aoisFeaturesAtom);
const newFeatures = features.filter(
(feature) => !ids.includes(feature.id as string)
);
set(aoisHashAtom, encodeAois(newFeatures));
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is an interesting paradigm. You have atoms that just update the main atom. Is this a normal jotai paradigm?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this normal? 😅
They are called derived atoms in Jotai. In my understanding, they extend the concept of selectors that exist in Redux or Recoil or many others, except they are not necessarily read-only (they can be write-only like here or read-write). The main idea is to use the URL hash as the single-source-of-truth, then provide a getter to get an array of AoIs, and setters to manipulate it.
I considered another alternative, which was to have a single read-write derived atom, then do various write operations in React land, but this turned out to be more concise. Happy to discuss if you wanna suggest alternatives.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just wondering about this. It is an interesting approach! :)

app/scripts/utils/polygon-url.ts Outdated Show resolved Hide resolved
@nerik nerik force-pushed the feature/exploration-aoi branch from 2f470a6 to dc479ae Compare October 10, 2023 13:22
@nerik nerik force-pushed the feature/exploration-aoi branch from dc479ae to 24dc07a Compare October 10, 2023 13:22
@nerik nerik force-pushed the feature/exploration-aoi branch from 719c7a4 to 2ab3dff Compare October 11, 2023 09:30
@nerik nerik requested a review from danielfdsilva October 11, 2023 09:31
@nerik nerik force-pushed the feature/exploration-aoi branch from 2ab3dff to 2b89c94 Compare October 11, 2023 09:43
@nerik nerik merged commit 7e42de9 into feature/exploration Oct 12, 2023
7 checks passed
@nerik nerik deleted the feature/exploration-aoi branch October 12, 2023 13:21
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