-
Notifications
You must be signed in to change notification settings - Fork 18
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
Refactor form building #55
Conversation
1145327
to
97b5a71
Compare
97b5a71
to
afe6cf0
Compare
e01841b
to
8b3f4f7
Compare
c2d4722
to
b0da52d
Compare
247f737
to
287e3d9
Compare
287e3d9
to
7f4f397
Compare
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.
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.
please update snapshots! |
I commented out the test for #66 |
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.
Thanks @martinRenou for the update (and the rework of geojson source creation)
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.
Is there a known reason for this update ?
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.
This is due to CSS changes in #48
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.
Maybe we should use a versioned version of the map if it exists.
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.
Indeed, that's a good idea. If that exists for one provider we should use it for testing.
Thanks! |
Generalize the form building logic.
This PR provides two types of React forms:
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