Skip to content

Commit

Permalink
chore: split umap.js in two modules (#2257)
Browse files Browse the repository at this point in the history 
Long awaited step! Very intrusive change!

The `umap.js` old papa js style is now moved to modules, and split in
two, as we did for layers and features: one module is for the data part,
and the other is for the rendering part, and this one inherits from
Leaflet.

Sadly, the split between those two modules is not as clear as I'd like
it to be, as some functions my be interpreted or not as rendering.

What is now moved to the rendering module is all elements that inherit
from Leaflet, so what concerns the map itself, but also controls in of
the Leaflet world (inheriting from L.Control). In the other hand, UI
elements that does not inherit from Leaflet are kept on the `umap.js`
module (panels, tooltip, contextmenu…).

Also, `Umap` as a `properties` key, to follow geojson spec, and
distinguish from `LeafletMap.options`.

This is a first step, that will need more work, but as tests pass, I'd
suggest to merge if we agree on the choices and continue working with
other (smaller) PRs (and I'll take care of rebasing current other PRs).

Some specific points I've in mind that does not smell good:
- umap.js module still uses some Leaflet utils
- Umap and LeafletMap have a reference to each other
- umap.js module still need Leaflet events
- layers and feature still need to have reference to both Umap and
LeafletMap
  • Loading branch information
@yohanboniface
yohanboniface authored Nov 13, 2024
2 parents ce3b855 + 378e0f3 commit 8c2a0ec
Show file tree
Hide file tree
Showing 38 changed files with 2,768 additions and 2,784 deletions.
12 changes: 0 additions & 12 deletions docs/dev/frontend.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,3 @@ When the data layers are initialized, they can have two states:
To mark what needs to be synced with the server, uMap currently mark objects as "dirty". Something marked as "dirty" has changed on the client, but is not yet saved on the server.

Each map, datalayer and permission objects can be marked as "dirty". When a change is made on an object, it will mark its parent as "dirty" as well, so it can be updated accordingly.

### Saving data to the server with `umap.save()`

Here is what's being done when you call `map.save()`:

1. `map.saveSelf()`, posting `name`, `center` and `settings` to the server, and then
2. calls `permission.save()`, which will post the permissions to the server, and then call back
3. `map.continueSaving()`, which will take the first dirtyLayer and call
4. `datalayer.save()` on it. It does the following:
1. Post the data (`name`, `displayOnLoad`, `rank`, `settings`, and `geojson`)
2. Calls `permission.save()`, posting `edit_status` to the server, and then calling `map.continue_saving()` and remove the datalayer from `dirtyDatalayers`.
5. When the `dirtyDatalayers` list is empty, we are done.
4 changes: 3 additions & 1 deletion umap/static/umap/js/components/fragment.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import Umap from '../modules/umap.js'

class UmapFragment extends HTMLElement {
connectedCallback() {
new U.Map(this.firstElementChild.id, JSON.parse(this.dataset.settings))
new Umap(this.firstElementChild.id, JSON.parse(this.dataset.settings))
}
}

Expand Down
39 changes: 20 additions & 19 deletions umap/static/umap/js/modules/browser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import { EXPORT_FORMATS } from './formatter.js'
import ContextMenu from './ui/contextmenu.js'

export default class Browser {
constructor(map) {
this.map = map
this.map.on('moveend', this.onMoveEnd, this)
constructor(umap, leafletMap) {
this._umap = umap
this._leafletMap = leafletMap
this._leafletMap.on('moveend', this.onMoveEnd, this)
this.options = {
filter: '',
inBbox: false,
Expand Down Expand Up @@ -82,7 +83,7 @@ export default class Browser {

updateDatalayer(datalayer) {
// Compute once, but use it for each feature later.
this.bounds = this.map.getBounds()
this.bounds = this._leafletMap.getBounds()
const parent = DomUtil.get(this.datalayerId(datalayer))
// Panel is not open
if (!parent) return
Expand Down Expand Up @@ -115,10 +116,10 @@ export default class Browser {
}

onFormChange() {
this.map.eachBrowsableDataLayer((datalayer) => {
this._umap.eachBrowsableDataLayer((datalayer) => {
datalayer.resetLayer(true)
this.updateDatalayer(datalayer)
if (this.map.fullPanel?.isOpen()) datalayer.tableEdit()
if (this._umap.fullPanel?.isOpen()) datalayer.tableEdit()
})
this.toggleBadge()
}
Expand All @@ -132,13 +133,13 @@ export default class Browser {
}

hasFilters() {
return !!this.options.filter || this.map.facets.isActive()
return !!this.options.filter || this._umap.facets.isActive()
}

onMoveEnd() {
if (!this.isOpen()) return
const isListDynamic = this.options.inBbox
this.map.eachBrowsableDataLayer((datalayer) => {
this._umap.eachBrowsableDataLayer((datalayer) => {
if (!isListDynamic && !datalayer.hasDynamicData()) return
this.updateDatalayer(datalayer)
})
Expand All @@ -147,7 +148,7 @@ export default class Browser {
update() {
if (!this.isOpen()) return
this.dataContainer.innerHTML = ''
this.map.eachBrowsableDataLayer((datalayer) => {
this._umap.eachBrowsableDataLayer((datalayer) => {
this.addDataLayer(datalayer, this.dataContainer)
})
}
Expand Down Expand Up @@ -186,9 +187,9 @@ export default class Browser {
DomEvent.on(builder.form, 'reset', () => {
window.setTimeout(builder.syncAll.bind(builder))
})
if (this.map.options.facetKey) {
fields = this.map.facets.build()
filtersBuilder = new L.FormBuilder(this.map.facets, fields, {
if (this._umap.properties.facetKey) {
fields = this._umap.facets.build()
filtersBuilder = new L.FormBuilder(this._umap.facets, fields, {
callback: () => this.onFormChange(),
})
DomEvent.on(filtersBuilder.form, 'reset', () => {
Expand All @@ -206,7 +207,7 @@ export default class Browser {
textContent: translate('Reset all'),
})

this.map.panel.open({
this._umap.panel.open({
content: container,
className: 'umap-browser',
})
Expand All @@ -230,7 +231,7 @@ export default class Browser {
`)
container.appendChild(toolbox)
toggle.addEventListener('click', () => this.toggleLayers())
fitBounds.addEventListener('click', () => this.map.fitDataBounds())
fitBounds.addEventListener('click', () => this._umap.fitDataBounds())
download.addEventListener('click', () => this.downloadVisible(download))
}

Expand All @@ -240,7 +241,7 @@ export default class Browser {
for (const format of Object.keys(EXPORT_FORMATS)) {
items.push({
label: format,
action: () => this.map.share.download(format),
action: () => this._umap.share.download(format),
})
}
menu.openBelow(element, items)
Expand All @@ -250,10 +251,10 @@ export default class Browser {
// If at least one layer is shown, hide it
// otherwise show all
let allHidden = true
this.map.eachBrowsableDataLayer((datalayer) => {
this._umap.eachBrowsableDataLayer((datalayer) => {
if (datalayer.isVisible()) allHidden = false
})
this.map.eachBrowsableDataLayer((datalayer) => {
this._umap.eachBrowsableDataLayer((datalayer) => {
if (allHidden) {
datalayer.show()
} else {
Expand All @@ -262,7 +263,7 @@ export default class Browser {
})
}

static backButton(map) {
static backButton(umap) {
const button = DomUtil.createButtonIcon(
DomUtil.create('li', '', undefined),
'icon-back',
Expand All @@ -271,7 +272,7 @@ export default class Browser {
// Fixme: remove me when this is merged and released
// https://github.com/Leaflet/Leaflet/pull/9052
DomEvent.disableClickPropagation(button)
DomEvent.on(button, 'click', map.openBrowser, map)
DomEvent.on(button, 'click', () => umap.openBrowser())
return button
}
}
43 changes: 21 additions & 22 deletions umap/static/umap/js/modules/caption.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ import { translate } from './i18n.js'
import * as Utils from './utils.js'

export default class Caption {
constructor(map) {
this.map = map
constructor(umap, leafletMap) {
this._umap = umap
this._leafletMap = leafletMap
}

isOpen() {
Expand All @@ -21,38 +22,36 @@ export default class Caption {
const hgroup = DomUtil.element({ tagName: 'hgroup', parent: container })
DomUtil.createTitle(
hgroup,
this.map.getDisplayName(),
this._umap.getDisplayName(),
'icon-caption icon-block',
'map-name'
)
this.map.addAuthorLink('h4', hgroup)
if (this.map.options.description) {
const title = Utils.loadTemplate('<h4></h4>')
hgroup.appendChild(title)
this._umap.addAuthorLink(title)
if (this._umap.properties.description) {
const description = DomUtil.element({
tagName: 'div',
className: 'umap-map-description text',
safeHTML: Utils.toHTML(this.map.options.description),
safeHTML: Utils.toHTML(this._umap.properties.description),
parent: container,
})
}
const datalayerContainer = DomUtil.create('div', 'datalayer-container', container)
this.map.eachDataLayerReverse((datalayer) =>
this._umap.eachDataLayerReverse((datalayer) =>
this.addDataLayer(datalayer, datalayerContainer)
)
const creditsContainer = DomUtil.create('div', 'credits-container', container)
this.addCredits(creditsContainer)
this.map.panel.open({ content: container }).then(() => {
this._umap.panel.open({ content: container }).then(() => {
// Create the legend when the panel is actually on the DOM
this.map.eachDataLayerReverse((datalayer) => datalayer.renderLegend())
this._umap.eachDataLayerReverse((datalayer) => datalayer.renderLegend())
})
}

addDataLayer(datalayer, container) {
if (!datalayer.options.inCaption) return
const p = DomUtil.create(
'p',
`caption-item ${datalayer.cssId}`,
container
)
const p = DomUtil.create('p', `caption-item ${datalayer.cssId}`, container)
const legend = DomUtil.create('span', 'datalayer-legend', p)
const headline = DomUtil.create('strong', '', p)
if (datalayer.options.description) {
Expand All @@ -69,16 +68,16 @@ export default class Caption {
addCredits(container) {
const credits = DomUtil.createFieldset(container, translate('Credits'))
let title = DomUtil.add('h5', '', credits, translate('User content credits'))
if (this.map.options.shortCredit || this.map.options.longCredit) {
if (this._umap.properties.shortCredit || this._umap.properties.longCredit) {
DomUtil.element({
tagName: 'p',
parent: credits,
safeHTML: Utils.toHTML(
this.map.options.longCredit || this.map.options.shortCredit
this._umap.properties.longCredit || this._umap.properties.shortCredit
),
})
}
if (this.map.options.licence) {
if (this._umap.properties.licence) {
const licence = DomUtil.add(
'p',
'',
Expand All @@ -88,8 +87,8 @@ export default class Caption {
DomUtil.createLink(
'',
licence,
this.map.options.licence.name,
this.map.options.licence.url
this._umap.properties.licence.name,
this._umap.properties.licence.url
)
} else {
DomUtil.add('p', '', credits, translate('No licence has been set'))
Expand All @@ -100,19 +99,19 @@ export default class Caption {
DomUtil.element({
tagName: 'strong',
parent: tilelayerCredit,
textContent: `${this.map.selected_tilelayer.options.name} `,
textContent: `${this._leafletMap.selectedTilelayer.options.name} `,
})
DomUtil.element({
tagName: 'span',
parent: tilelayerCredit,
safeHTML: this.map.selected_tilelayer.getAttribution(),
safeHTML: this._leafletMap.selectedTilelayer.getAttribution(),
})
const urls = {
leaflet: 'http://leafletjs.com',
django: 'https://www.djangoproject.com',
umap: 'https://umap-project.org/',
changelog: 'https://docs.umap-project.org/en/master/changelog/',
version: this.map.options.umap_version,
version: this._umap.properties.umap_version,
}
const creditHTML = translate(
`
Expand Down
Loading

0 comments on commit 8c2a0ec

Please sign in to comment.