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

Template-based importer (vs. using L.DomUtil) #1461

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Conversation

davidbgk
Copy link
Contributor

After a discussion with @almet we realized that it would be possible to write HTML in templates instead of building everything on the JS side using L.DomUtil, this is a first small step to verify that it's both possible and pertinent.

There are still some rough edges but let's discuss the direction in itself first!

@yohanboniface
Copy link
Member

Templates are tempting, but I feel like until we need all those querySelector the final benefit is small, while it also adds a small indirection / complexity (declaring HTML in some file, and using it in another).
Also, given Leaflet itself is programmatic, this means introducing another pattern alongside.
I liked your experiment much as a direction, even if of course the step would be much bigger.
At this stage, the direction is not totally clear to me.

@davidbgk
Copy link
Contributor Author

At this stage, the direction is not totally clear to me.

We might want to go a step further and turn that form into a web component in itself which is able to do its stuff (select build from data, checks, auto-selected fields on upload) on itself prior to dispatch a custom event with the result to uMap's core JS.

I'd love to give it a try 😇

@yohanboniface
Copy link
Member

Sounds appealing! But what would be the browser support ?

@davidbgk
Copy link
Contributor Author

Custom elements are not fully baseline compliant :'( But it's coming!

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