Skip to content

Commit

Permalink
chore: internalize FormBuilder (#2420)
Browse files Browse the repository at this point in the history
fix #2280 

That's a first step, which:
- internalize Formbuilder as a bunch of modules
- use Javascript classes instead of Leaflet ones
- remove dependencies to Leaflet (L.DomUtil…)
- replaces `L.FormBuilder` by `Form` (in theory generic, but not quite)
and `U.FormBuilder` by `MutatingForm` (knows about isDirty,
`inheritable` and such)

There is much more room for refactor, but let's do it step by step!
  • Loading branch information
yohanboniface authored Jan 11, 2025
2 parents 86a8e17 + 07c29ab commit f53d435
Show file tree
Hide file tree
Showing 23 changed files with 1,843 additions and 1,824 deletions.
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
"leaflet": "1.9.4",
"leaflet-editable": "^1.3.0",
"leaflet-editinosm": "0.2.3",
"leaflet-formbuilder": "0.2.10",
"leaflet-fullscreen": "1.0.2",
"leaflet-hash": "0.2.1",
"leaflet-i18n": "0.3.5",
Expand Down
1 change: 0 additions & 1 deletion scripts/vendorsjs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ mkdir -p umap/static/umap/vendors/markercluster/ && cp -r node_modules/leaflet.m
mkdir -p umap/static/umap/vendors/heat/ && cp -r node_modules/leaflet.heat/dist/leaflet-heat.js umap/static/umap/vendors/heat/
mkdir -p umap/static/umap/vendors/fullscreen/ && cp -r node_modules/leaflet-fullscreen/dist/** umap/static/umap/vendors/fullscreen/
mkdir -p umap/static/umap/vendors/toolbar/ && cp -r node_modules/leaflet-toolbar/dist/leaflet.toolbar.* umap/static/umap/vendors/toolbar/
mkdir -p umap/static/umap/vendors/formbuilder/ && cp -r node_modules/leaflet-formbuilder/Leaflet.FormBuilder.js umap/static/umap/vendors/formbuilder/
mkdir -p umap/static/umap/vendors/measurable/ && cp -r node_modules/leaflet-measurable/Leaflet.Measurable.* umap/static/umap/vendors/measurable/
mkdir -p umap/static/umap/vendors/photon/ && cp -r node_modules/leaflet.photon/leaflet.photon.js umap/static/umap/vendors/photon/
mkdir -p umap/static/umap/vendors/csv2geojson/ && cp -r node_modules/csv2geojson/csv2geojson.js umap/static/umap/vendors/csv2geojson/
Expand Down
31 changes: 13 additions & 18 deletions umap/static/umap/css/form.css
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
.umap-form-inline .formbox,
.umap-form-inline {
display: inline;
}
Expand Down Expand Up @@ -381,16 +382,19 @@ input.switch:checked ~ label:after {
box-shadow: inset 0 0 6px 0px #2c3233;
color: var(--color-darkGray);
}
.inheritable .header,
.inheritable {
clear: both;
overflow: hidden;
.inheritable .header .buttons {
padding: 0;
}
.inheritable .header {
margin-bottom: 5px;
display: flex;
align-items: center;
align-content: center;
justify-content: space-between;
}
.inheritable .header label {
padding-top: 6px;
width: initial;
}
.inheritable + .inheritable {
border-top: 1px solid #222;
Expand All @@ -400,22 +404,11 @@ input.switch:checked ~ label:after {
.umap-field-iconUrl .action-button,
.inheritable .define,
.inheritable .undefine {
float: inline-end;
width: initial;
min-height: 18px;
line-height: 18px;
margin-bottom: 0;
}
.inheritable .quick-actions {
float: inline-end;
}
.inheritable .quick-actions .formbox {
margin-bottom: 0;
}
.inheritable .quick-actions input {
width: 100px;
margin-inline-end: 5px;
}
.inheritable .define,
.inheritable.undefined .undefine,
.inheritable.undefined .show-on-defined {
Expand Down Expand Up @@ -493,12 +486,15 @@ i.info {
padding: 0 5px;
}
.flat-tabs {
display: flex;
display: none;
justify-content: space-around;
font-size: 1.2em;
margin-bottom: 20px;
border-bottom: 1px solid #bebebe;
}
.flat-tabs:has(.flat) {
display: flex;
}
.flat-tabs button {
padding: 10px;
text-decoration: none;
Expand Down Expand Up @@ -534,7 +530,7 @@ i.info {
background-color: #999;
text-align: center;
margin-bottom: 5px;
display: block;
display: inline-block;
color: black;
font-weight: bold;
}
Expand All @@ -559,7 +555,6 @@ i.info {
clear: both;
margin-bottom: 20px;
overflow: hidden;
display: none;
}
.umap-color-picker span {
width: 20px;
Expand Down
11 changes: 5 additions & 6 deletions umap/static/umap/js/modules/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as Icon from './rendering/icon.js'
import * as Utils from './utils.js'
import { EXPORT_FORMATS } from './formatter.js'
import ContextMenu from './ui/contextmenu.js'
import { Form } from './form/builder.js'

export default class Browser {
constructor(umap, leafletMap) {
Expand Down Expand Up @@ -179,19 +180,17 @@ export default class Browser {
],
['options.inBbox', { handler: 'Switch', label: translate('Current map view') }],
]
const builder = new L.FormBuilder(this, fields, {
callback: () => this.onFormChange(),
})
const builder = new Form(this, fields)
builder.on('set', () => this.onFormChange())
let filtersBuilder
this.formContainer.appendChild(builder.build())
DomEvent.on(builder.form, 'reset', () => {
window.setTimeout(builder.syncAll.bind(builder))
})
if (this._umap.properties.facetKey) {
fields = this._umap.facets.build()
filtersBuilder = new L.FormBuilder(this._umap.facets, fields, {
callback: () => this.onFormChange(),
})
filtersBuilder = new Form(this._umap.facets, fields)
filtersBuilder.on('set', () => this.onFormChange())
DomEvent.on(filtersBuilder.form, 'reset', () => {
window.setTimeout(filtersBuilder.syncAll.bind(filtersBuilder))
})
Expand Down
42 changes: 19 additions & 23 deletions umap/static/umap/js/modules/data/features.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
MaskPolygon,
} from '../rendering/ui.js'
import loadPopup from '../rendering/popup.js'
import { MutatingForm } from '../form/builder.js'

class Feature {
constructor(umap, datalayer, geojson = {}, id = null) {
Expand Down Expand Up @@ -225,15 +226,11 @@ class Feature {
`icon-${this.getClassName()}`
)

let builder = new U.FormBuilder(
this,
[['datalayer', { handler: 'DataLayerSwitcher' }]],
{
callback() {
this.edit(event)
}, // removeLayer step will close the edit panel, let's reopen it
}
)
let builder = new MutatingForm(this, [
['datalayer', { handler: 'DataLayerSwitcher' }],
])
// removeLayer step will close the edit panel, let's reopen it
builder.on('set', () => this.edit(event))
container.appendChild(builder.build())

const properties = []
Expand All @@ -254,7 +251,7 @@ class Feature {
labelKeyFound = U.DEFAULT_LABEL_KEY
}
properties.unshift([`properties.${labelKeyFound}`, { label: labelKeyFound }])
builder = new U.FormBuilder(this, properties, {
builder = new MutatingForm(this, properties, {
id: 'umap-feature-properties',
})
container.appendChild(builder.build())
Expand Down Expand Up @@ -285,7 +282,7 @@ class Feature {

appendEditFieldsets(container) {
const optionsFields = this.getShapeOptions()
let builder = new U.FormBuilder(this, optionsFields, {
let builder = new MutatingForm(this, optionsFields, {
id: 'umap-feature-shape-properties',
})
const shapeProperties = DomUtil.createFieldset(
Expand All @@ -295,7 +292,7 @@ class Feature {
shapeProperties.appendChild(builder.build())

const advancedOptions = this.getAdvancedOptions()
builder = new U.FormBuilder(this, advancedOptions, {
builder = new MutatingForm(this, advancedOptions, {
id: 'umap-feature-advanced-properties',
})
const advancedProperties = DomUtil.createFieldset(
Expand All @@ -305,7 +302,7 @@ class Feature {
advancedProperties.appendChild(builder.build())

const interactionOptions = this.getInteractionOptions()
builder = new U.FormBuilder(this, interactionOptions)
builder = new MutatingForm(this, interactionOptions)
const popupFieldset = DomUtil.createFieldset(
container,
translate('Interaction options')
Expand Down Expand Up @@ -733,16 +730,15 @@ export class Point extends Feature {
['ui._latlng.lat', { handler: 'FloatInput', label: translate('Latitude') }],
['ui._latlng.lng', { handler: 'FloatInput', label: translate('Longitude') }],
]
const builder = new U.FormBuilder(this, coordinatesOptions, {
callback: () => {
if (!this.ui._latlng.isValid()) {
Alert.error(translate('Invalid latitude or longitude'))
builder.restoreField('ui._latlng.lat')
builder.restoreField('ui._latlng.lng')
}
this.pullGeometry()
this.zoomTo({ easing: false })
},
const builder = new MutatingForm(this, coordinatesOptions)
builder.on('set', () => {
if (!this.ui._latlng.isValid()) {
Alert.error(translate('Invalid latitude or longitude'))
builder.restoreField('ui._latlng.lat')
builder.restoreField('ui._latlng.lng')
}
this.pullGeometry()
this.zoomTo({ easing: false })
})
const fieldset = DomUtil.createFieldset(container, translate('Coordinates'))
fieldset.appendChild(builder.build())
Expand Down
28 changes: 13 additions & 15 deletions umap/static/umap/js/modules/data/layer.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
// Uses U.FormBuilder not available as ESM

// FIXME: this module should not depend on Leaflet
import {
DomUtil,
Expand All @@ -22,6 +20,7 @@ import { Point, LineString, Polygon } from './features.js'
import TableEditor from '../tableeditor.js'
import { ServerStored } from '../saving.js'
import * as Schema from '../schema.js'
import { MutatingForm } from '../form/builder.js'

export const LAYER_TYPES = [
DefaultLayer,
Expand Down Expand Up @@ -659,7 +658,7 @@ export class DataLayer extends ServerStored {
{
label: translate('Data is browsable'),
handler: 'Switch',
helpEntries: 'browsable',
helpEntries: ['browsable'],
},
],
[
Expand All @@ -671,20 +670,19 @@ export class DataLayer extends ServerStored {
],
]
DomUtil.createTitle(container, translate('Layer properties'), 'icon-layers')
let builder = new U.FormBuilder(this, metadataFields, {
callback(e) {
this._umap.onDataLayersChanged()
if (e.helper.field === 'options.type') {
this.edit()
}
},
let builder = new MutatingForm(this, metadataFields)
builder.on('set', ({ detail }) => {
this._umap.onDataLayersChanged()
if (detail.helper.field === 'options.type') {
this.edit()
}
})
container.appendChild(builder.build())

const layerOptions = this.layer.getEditableOptions()

if (layerOptions.length) {
builder = new U.FormBuilder(this, layerOptions, {
builder = new MutatingForm(this, layerOptions, {
id: 'datalayer-layer-properties',
})
const layerProperties = DomUtil.createFieldset(
Expand All @@ -707,7 +705,7 @@ export class DataLayer extends ServerStored {
'options.fillOpacity',
]

builder = new U.FormBuilder(this, shapeOptions, {
builder = new MutatingForm(this, shapeOptions, {
id: 'datalayer-advanced-properties',
})
const shapeProperties = DomUtil.createFieldset(
Expand All @@ -724,7 +722,7 @@ export class DataLayer extends ServerStored {
'options.toZoom',
]

builder = new U.FormBuilder(this, optionsFields, {
builder = new MutatingForm(this, optionsFields, {
id: 'datalayer-advanced-properties',
})
const advancedProperties = DomUtil.createFieldset(
Expand All @@ -743,7 +741,7 @@ export class DataLayer extends ServerStored {
'options.outlinkTarget',
'options.interactive',
]
builder = new U.FormBuilder(this, popupFields)
builder = new MutatingForm(this, popupFields)
const popupFieldset = DomUtil.createFieldset(
container,
translate('Interaction options')
Expand Down Expand Up @@ -799,7 +797,7 @@ export class DataLayer extends ServerStored {
container,
translate('Remote data')
)
builder = new U.FormBuilder(this, remoteDataFields)
builder = new MutatingForm(this, remoteDataFields)
remoteDataContainer.appendChild(builder.build())
DomUtil.createButton(
'button umap-verify',
Expand Down
Loading

0 comments on commit f53d435

Please sign in to comment.