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

More modular fields code #3914

Closed
bhousel opened this issue Mar 20, 2017 · 4 comments
Closed

More modular fields code #3914

bhousel opened this issue Mar 20, 2017 · 4 comments
Labels
field An issue with a field in the user interface

Comments

@bhousel
Copy link
Member

bhousel commented Mar 20, 2017

We should probably improve how we handle fields in the iD codebase. This is something that @kepta and I were talking about recently.

All of our fields are currently written in a way that you can't just create a field, it's expected that presets "own" fields. The problem with this is when we want to create a special field inside some other field, the only way to do this is to duplicate a lot of code.

This impacts the bridge/tunnel radio field #3911 and @kepta's lane editor #3822, and whatever other "advanced" fields we want to create. In the case of the #3911, just to add a "Type" combo, I had to copy over a lot of the code from combo.js. This code duplication adds a bunch of technical debt, and we should clean it up once we find a better way.

We can probably fix this by splitting this UIField constructor (factory?) out into its own module, making sure to account for all the preset closure variables that it depends on (e.g. it should have its own dispatch, etc). We might be able to do this without a major semver bump.

@bhousel bhousel added the field An issue with a field in the user interface label Mar 20, 2017
@kepta
Copy link
Collaborator

kepta commented Mar 20, 2017

@bhousel I really like the idea. After working on #3822 I realised we definitely need something like this.
It would be great if we could chalk out the list of things we would need to do for this refactor.
And also I was wonder for this new refactor will we still have the
custom events or should we look into other design patterns, since we are changing the whole submodule.

@bhousel bhousel added the priority A top priority issue that has a big impact or matter most to new mappers label Mar 26, 2017
@bhousel bhousel added the wip Work in progress label Aug 2, 2017
@bhousel bhousel mentioned this issue Aug 3, 2017
@bhousel
Copy link
Member Author

bhousel commented Aug 4, 2017

A field in a field.. This is cool that it works.

I liked the previous trimmed down design of this layer field, so I will make sure we can construct fields without the label, frame, form buttons if we want to.

screenshot 2017-08-04 15 35 05

@kepta
Copy link
Collaborator

kepta commented Aug 5, 2017

This is amazing @bhousel !!

@bhousel
Copy link
Member Author

bhousel commented Aug 7, 2017

Done in #4214 😄

@bhousel bhousel closed this as completed Aug 7, 2017
@bhousel bhousel removed wip Work in progress priority A top priority issue that has a big impact or matter most to new mappers labels Aug 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
field An issue with a field in the user interface
Projects
None yet
Development

No branches or pull requests

2 participants