Skip to content

Commit

Permalink
fix: always unset editedFeature on editPanel close (#2237)
Browse files Browse the repository at this point in the history
And only this panel.

This was creating a weird bug, steps to reproduce:

- create a marker
- shift-click on the marker to edit the layer (so without explicitly
closing the panel)
- try to type the layer name: the panel would close

This is also because currently the schema and render() are too dump, and
when any `name` is changed then the `data` reflow is called, while it
should not when editing the datalayer name.

We want to:
- have more targeted schema
- have more specific reflow in render

But that's for other PRs!
  • Loading branch information
yohanboniface authored Oct 25, 2024
2 parents 023f48e + b36c03a commit a16f6d4
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 15 deletions.
4 changes: 3 additions & 1 deletion umap/static/umap/js/modules/rendering/ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ const FeatureMixin = {
if (map.editedFeature === this.feature) {
this.feature._marked_for_deletion = true
this.feature.endEdit()
map.editPanel.close()
if (map.editedFeature === this.feature) {
map.editPanel.close()
}
}
},

Expand Down
13 changes: 12 additions & 1 deletion umap/static/umap/js/modules/ui/panel.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ export class Panel {
}

open({ content, className, actions = [] } = {}) {
if (this.isOpen()) {
this.onClose()
}
this.container.className = `with-transition panel window ${this.className} ${
this.mode || ''
}`
Expand Down Expand Up @@ -71,10 +74,13 @@ export class Panel {

close() {
document.body.classList.remove(`panel-${this.className.split(' ')[0]}-on`)
this.onClose()
}

onClose() {
if (DomUtil.hasClass(this.container, 'on')) {
DomUtil.removeClass(this.container, 'on')
this.map.invalidateSize({ pan: false })
this.map.editedFeature = null
}
}
}
Expand All @@ -84,6 +90,11 @@ export class EditPanel extends Panel {
super(map)
this.className = 'right dark'
}

onClose() {
super.onClose()
this.map.editedFeature = null
}
}

export class FullPanel extends Panel {
Expand Down
28 changes: 16 additions & 12 deletions umap/static/umap/js/umap.controls.js
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,7 @@ const ControlsMixin = {
const connectedPeersCount = L.DomUtil.createButton(
'leaflet-control-connected-peers',
rightContainer,
'',
''
)
L.DomEvent.on(connectedPeersCount, 'mouseover', () => {
this.tooltip.open({
Expand Down Expand Up @@ -1182,17 +1182,6 @@ U.Editable = L.Editable.extend({
initialize: function (map, options) {
L.Editable.prototype.initialize.call(this, map, options)
this.on('editable:drawing:click editable:drawing:move', this.drawingTooltip)
this.on('editable:drawing:end', (event) => {
this.map.tooltip.close()
// Leaflet.Editable will delete the drawn shape if invalid
// (eg. line has only one drawn point)
// So let's check if the layer has no more shape
if (!event.layer.feature.hasGeom()) {
event.layer.feature.del()
} else {
event.layer.feature.edit()
}
})
// Layer for items added by users
this.on('editable:drawing:cancel', (event) => {
if (event.layer instanceof U.LeafletMarker) event.layer.feature.del()
Expand Down Expand Up @@ -1322,4 +1311,19 @@ U.Editable = L.Editable.extend({
L.DomEvent.stop(e)
e.cancel()
},

onEscape: function () {
this.once('editable:drawing:end', (event) => {
this.map.tooltip.close()
// Leaflet.Editable will delete the drawn shape if invalid
// (eg. line has only one drawn point)
// So let's check if the layer has no more shape
if (!event.layer.feature.hasGeom()) {
event.layer.feature.del()
} else {
event.layer.feature.edit()
}
})
this.stopDrawing()
},
})
2 changes: 1 addition & 1 deletion umap/static/umap/js/umap.js
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ U.Map = L.Map.extend({
if (this.importer.dialog.visible) {
this.importer.dialog.close()
} else if (this.editEnabled && this.editTools.drawing()) {
this.editTools.stopDrawing()
this.editTools.onEscape()
} else if (this.measureTools.enabled()) {
this.measureTools.stopDrawing()
} else if (this.fullPanel?.isOpen()) {
Expand Down

0 comments on commit a16f6d4

Please sign in to comment.