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

Refactor form building #55

Merged
merged 17 commits into from
Jul 22, 2024

Conversation

martinRenou
Copy link
Member

@martinRenou martinRenou commented Jul 16, 2024

Generalize the form building logic.

This PR provides two types of React forms:

  • creation form: Allows to create a layer and a source at once or just one or the other
  • edit form: Allows to edit a layer and a source at once or just one or the other

These react forms are used in multiple places of the UI (dialogs, gallery, right panel)

Specializing edit/create UI requires providing a custom form class inheriting from BaseForm

Copy link
Contributor

Binder 👈 Launch a Binder on branch martinRenou/jupytergis/refactor_form_building

@martinRenou martinRenou force-pushed the refactor_form_building branch 2 times, most recently from 1145327 to 97b5a71 Compare July 19, 2024 10:06
@martinRenou martinRenou force-pushed the refactor_form_building branch from 97b5a71 to afe6cf0 Compare July 19, 2024 11:11
@martinRenou martinRenou force-pushed the refactor_form_building branch from e01841b to 8b3f4f7 Compare July 19, 2024 11:54
@martinRenou martinRenou force-pushed the refactor_form_building branch from c2d4722 to b0da52d Compare July 19, 2024 14:06
@martinRenou martinRenou marked this pull request as ready for review July 19, 2024 14:07
@martinRenou martinRenou force-pushed the refactor_form_building branch from 247f737 to 287e3d9 Compare July 19, 2024 14:19
@martinRenou martinRenou force-pushed the refactor_form_building branch from 287e3d9 to 7f4f397 Compare July 19, 2024 14:22
@martinRenou martinRenou requested a review from brichet July 19, 2024 14:46
Copy link
Collaborator

@brichet brichet left a comment

Choose a reason for hiding this comment

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

Thanks @martinRenou, huge changes, and it works well.

I have a few comments below, otherwise let's fix the tests and move forward.

For the record, we should also update the createGeoJSONSource() and createVectorLayer() functions, which do not use this new form builder.

packages/base/src/panelview/objectproperties.tsx Outdated Show resolved Hide resolved
packages/base/src/formbuilder/objectform/geojsonsource.ts Outdated Show resolved Hide resolved
@martinRenou martinRenou requested a review from brichet July 22, 2024 10:10
@martinRenou
Copy link
Member Author

martinRenou commented Jul 22, 2024

please update snapshots!

@martinRenou
Copy link
Member Author

I commented out the test for #66

@martinRenou martinRenou reopened this Jul 22, 2024
Copy link
Collaborator

@brichet brichet left a comment

Choose a reason for hiding this comment

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

Thanks @martinRenou for the update (and the rework of geojson source creation)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a known reason for this update ?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is due to CSS changes in #48

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we should use a versioned version of the map if it exists.

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, that's a good idea. If that exists for one provider we should use it for testing.

@martinRenou martinRenou merged commit ab33f5b into geojupyter:main Jul 22, 2024
8 checks passed
@martinRenou martinRenou deleted the refactor_form_building branch July 22, 2024 12:40
@martinRenou
Copy link
Member Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants