From cba0af96f63a2ac63b1a708679f747514b2a39d1 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Sat, 5 Oct 2024 08:55:56 +0200 Subject: [PATCH 1/8] fix: fix contextmenu positionning when map is not full screen --- umap/static/umap/js/modules/ui/base.js | 10 ++++++---- umap/static/umap/js/modules/ui/contextmenu.js | 5 +++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/umap/static/umap/js/modules/ui/base.js b/umap/static/umap/js/modules/ui/base.js index 725e3c9b..7fa21748 100644 --- a/umap/static/umap/js/modules/ui/base.js +++ b/umap/static/umap/js/modules/ui/base.js @@ -63,16 +63,18 @@ export class Positioned { else this.container.style.bottom = 'initial' } - computePosition([x, y]) { + computePosition([x, y], parent) { let left let top - if (x < window.innerWidth / 2) { + x = x - parent.getBoundingClientRect().left + y = y - parent.getBoundingClientRect().top + if (x < parent.offsetWidth / 2) { left = x } else { left = x - this.container.offsetWidth } - if (y < window.innerHeight / 2) { - top = Math.min(y, window.innerHeight - this.container.offsetHeight) + if (y < parent.offsetHeight / 2) { + top = Math.min(y, parent.offsetHeight - this.container.offsetHeight) } else { top = Math.max(0, y - this.container.offsetHeight) } diff --git a/umap/static/umap/js/modules/ui/contextmenu.js b/umap/static/umap/js/modules/ui/contextmenu.js index fd61c84a..305daea1 100644 --- a/umap/static/umap/js/modules/ui/contextmenu.js +++ b/umap/static/umap/js/modules/ui/contextmenu.js @@ -36,11 +36,12 @@ export default class ContextMenu extends Positioned { this.container.appendChild(li) } } - document.body.appendChild(this.container) + const parent = document.elementFromPoint(left, top).closest('.leaflet-container') + parent.appendChild(this.container) if (this.options.fixed) { this.setPosition({ left, top }) } else { - this.computePosition([left, top]) + this.computePosition([left, top], parent) } this.container.querySelector('li > *').focus() this.container.addEventListener( From c4b80afb15a279331ec660f0e2774364be4ea591 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Sat, 5 Oct 2024 10:38:51 +0200 Subject: [PATCH 2/8] fix: simpler way to deal with positionning contextmenu Use pageX/Y from the begginning instead of recomputing it later. --- umap/static/umap/js/modules/rendering/ui.js | 2 +- umap/static/umap/js/modules/tableeditor.js | 2 +- umap/static/umap/js/modules/ui/base.js | 10 ++++------ umap/static/umap/js/modules/ui/contextmenu.js | 5 ++--- umap/static/umap/js/umap.js | 2 +- 5 files changed, 9 insertions(+), 12 deletions(-) diff --git a/umap/static/umap/js/modules/rendering/ui.js b/umap/static/umap/js/modules/rendering/ui.js index 0311cce4..ffa4fe73 100644 --- a/umap/static/umap/js/modules/rendering/ui.js +++ b/umap/static/umap/js/modules/rendering/ui.js @@ -89,7 +89,7 @@ const FeatureMixin = { const items = this._map.getContextMenuItems(event) items.push('-', ...this.feature.getContextMenuItems(event)) this._map.contextmenu.open( - [event.originalEvent.clientX, event.originalEvent.clientY], + [event.originalEvent.pageX, event.originalEvent.pageY], items ) }, diff --git a/umap/static/umap/js/modules/tableeditor.js b/umap/static/umap/js/modules/tableeditor.js index 630360f1..7c13f5ad 100644 --- a/umap/static/umap/js/modules/tableeditor.js +++ b/umap/static/umap/js/modules/tableeditor.js @@ -64,7 +64,7 @@ export default class TableEditor extends WithTemplate { action: () => this.deleteProperty(property), }) } - this.contextmenu.open([event.clientX, event.clientY], actions) + this.contextmenu.open([event.pageX, event.pageY], actions) } renderHeaders() { diff --git a/umap/static/umap/js/modules/ui/base.js b/umap/static/umap/js/modules/ui/base.js index 7fa21748..67f17b0f 100644 --- a/umap/static/umap/js/modules/ui/base.js +++ b/umap/static/umap/js/modules/ui/base.js @@ -63,18 +63,16 @@ export class Positioned { else this.container.style.bottom = 'initial' } - computePosition([x, y], parent) { + computePosition([x, y]) { let left let top - x = x - parent.getBoundingClientRect().left - y = y - parent.getBoundingClientRect().top - if (x < parent.offsetWidth / 2) { + if (x < window.offsetWidth / 2) { left = x } else { left = x - this.container.offsetWidth } - if (y < parent.offsetHeight / 2) { - top = Math.min(y, parent.offsetHeight - this.container.offsetHeight) + if (y < window.offsetHeight / 2) { + top = Math.min(y, window.offsetHeight - this.container.offsetHeight) } else { top = Math.max(0, y - this.container.offsetHeight) } diff --git a/umap/static/umap/js/modules/ui/contextmenu.js b/umap/static/umap/js/modules/ui/contextmenu.js index 305daea1..fd61c84a 100644 --- a/umap/static/umap/js/modules/ui/contextmenu.js +++ b/umap/static/umap/js/modules/ui/contextmenu.js @@ -36,12 +36,11 @@ export default class ContextMenu extends Positioned { this.container.appendChild(li) } } - const parent = document.elementFromPoint(left, top).closest('.leaflet-container') - parent.appendChild(this.container) + document.body.appendChild(this.container) if (this.options.fixed) { this.setPosition({ left, top }) } else { - this.computePosition([left, top], parent) + this.computePosition([left, top]) } this.container.querySelector('li > *').focus() this.container.addEventListener( diff --git a/umap/static/umap/js/umap.js b/umap/static/umap/js/umap.js index cfc2c018..8057ec2c 100644 --- a/umap/static/umap/js/umap.js +++ b/umap/static/umap/js/umap.js @@ -1773,7 +1773,7 @@ U.Map = L.Map.extend({ onContextMenu: function (event) { const items = this.getContextMenuItems(event) this.contextmenu.open( - [event.originalEvent.clientX, event.originalEvent.clientY], + [event.originalEvent.pageX, event.originalEvent.pageY], items ) }, From 42e7fb8daee85c2e6681430082829c6d825041d6 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Sat, 5 Oct 2024 10:41:33 +0200 Subject: [PATCH 3/8] chore: pass event to ContextMenu.open --- umap/static/umap/js/modules/rendering/ui.js | 5 +---- umap/static/umap/js/modules/tableeditor.js | 2 +- umap/static/umap/js/modules/ui/contextmenu.js | 4 +++- umap/static/umap/js/umap.js | 5 +---- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/umap/static/umap/js/modules/rendering/ui.js b/umap/static/umap/js/modules/rendering/ui.js index ffa4fe73..034962ae 100644 --- a/umap/static/umap/js/modules/rendering/ui.js +++ b/umap/static/umap/js/modules/rendering/ui.js @@ -88,10 +88,7 @@ const FeatureMixin = { DomEvent.stop(event) const items = this._map.getContextMenuItems(event) items.push('-', ...this.feature.getContextMenuItems(event)) - this._map.contextmenu.open( - [event.originalEvent.pageX, event.originalEvent.pageY], - items - ) + this._map.contextmenu.open(event.originalEvent, items) }, onCommit: function () { diff --git a/umap/static/umap/js/modules/tableeditor.js b/umap/static/umap/js/modules/tableeditor.js index 7c13f5ad..79fb6c54 100644 --- a/umap/static/umap/js/modules/tableeditor.js +++ b/umap/static/umap/js/modules/tableeditor.js @@ -64,7 +64,7 @@ export default class TableEditor extends WithTemplate { action: () => this.deleteProperty(property), }) } - this.contextmenu.open([event.pageX, event.pageY], actions) + this.contextmenu.open(event, actions) } renderHeaders() { diff --git a/umap/static/umap/js/modules/ui/contextmenu.js b/umap/static/umap/js/modules/ui/contextmenu.js index fd61c84a..f02e2b1a 100644 --- a/umap/static/umap/js/modules/ui/contextmenu.js +++ b/umap/static/umap/js/modules/ui/contextmenu.js @@ -15,7 +15,9 @@ export default class ContextMenu extends Positioned { }) } - open([left, top], items) { + open(event, items) { + const left = event.pageX + const top = event.pageY this.container.innerHTML = '' for (const item of items) { if (item === '-') { diff --git a/umap/static/umap/js/umap.js b/umap/static/umap/js/umap.js index 8057ec2c..5b5737d4 100644 --- a/umap/static/umap/js/umap.js +++ b/umap/static/umap/js/umap.js @@ -1772,10 +1772,7 @@ U.Map = L.Map.extend({ onContextMenu: function (event) { const items = this.getContextMenuItems(event) - this.contextmenu.open( - [event.originalEvent.pageX, event.originalEvent.pageY], - items - ) + this.contextmenu.open(event.originalEvent, items) }, editInOSM: function (e) { From 89c13a3b7eb028758382a679e2f21f56cd2c45de Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Sat, 5 Oct 2024 11:08:36 +0200 Subject: [PATCH 4/8] fix: window.offsetHeight does not means anything --- umap/static/umap/js/modules/ui/base.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/umap/static/umap/js/modules/ui/base.js b/umap/static/umap/js/modules/ui/base.js index 67f17b0f..725e3c9b 100644 --- a/umap/static/umap/js/modules/ui/base.js +++ b/umap/static/umap/js/modules/ui/base.js @@ -66,13 +66,13 @@ export class Positioned { computePosition([x, y]) { let left let top - if (x < window.offsetWidth / 2) { + if (x < window.innerWidth / 2) { left = x } else { left = x - this.container.offsetWidth } - if (y < window.offsetHeight / 2) { - top = Math.min(y, window.offsetHeight - this.container.offsetHeight) + if (y < window.innerHeight / 2) { + top = Math.min(y, window.innerHeight - this.container.offsetHeight) } else { top = Math.max(0, y - this.container.offsetHeight) } From 70f06e78527ec9bfedb566d165f629956d54030e Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Sat, 5 Oct 2024 11:08:50 +0200 Subject: [PATCH 5/8] fix: insert contextmenu in the offsetParent Otherwise when parent is a dialog, if the the contextemenu is in the body, it will be below the dialog forever --- umap/static/umap/js/modules/ui/contextmenu.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/umap/static/umap/js/modules/ui/contextmenu.js b/umap/static/umap/js/modules/ui/contextmenu.js index f02e2b1a..d385e492 100644 --- a/umap/static/umap/js/modules/ui/contextmenu.js +++ b/umap/static/umap/js/modules/ui/contextmenu.js @@ -38,7 +38,8 @@ export default class ContextMenu extends Positioned { this.container.appendChild(li) } } - document.body.appendChild(this.container) + const parent = document.elementFromPoint(event.clientX, event.clientY).offsetParent + parent.appendChild(this.container) if (this.options.fixed) { this.setPosition({ left, top }) } else { From c844e1c03d4c84c7f427473446d8acd286e5277f Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Sat, 5 Oct 2024 11:22:02 +0200 Subject: [PATCH 6/8] chore: use position: fixed for contextmenu --- umap/static/umap/css/contextmenu.css | 2 +- umap/static/umap/js/modules/ui/contextmenu.js | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/umap/static/umap/css/contextmenu.css b/umap/static/umap/css/contextmenu.css index e7ed2508..79b55c09 100644 --- a/umap/static/umap/css/contextmenu.css +++ b/umap/static/umap/css/contextmenu.css @@ -1,7 +1,7 @@ .umap-contextmenu { background-color: var(--background-color); padding: calc(var(--box-padding) / 2) var(--box-padding); - position: absolute; + position: fixed; z-index: var(--zindex-contextmenu); border-radius: var(--border-radius); box-shadow: var(--block-shadow); diff --git a/umap/static/umap/js/modules/ui/contextmenu.js b/umap/static/umap/js/modules/ui/contextmenu.js index d385e492..e85c562b 100644 --- a/umap/static/umap/js/modules/ui/contextmenu.js +++ b/umap/static/umap/js/modules/ui/contextmenu.js @@ -16,8 +16,8 @@ export default class ContextMenu extends Positioned { } open(event, items) { - const left = event.pageX - const top = event.pageY + const left = event.clientX + const top = event.clientY this.container.innerHTML = '' for (const item of items) { if (item === '-') { @@ -38,7 +38,7 @@ export default class ContextMenu extends Positioned { this.container.appendChild(li) } } - const parent = document.elementFromPoint(event.clientX, event.clientY).offsetParent + const parent = document.elementFromPoint(left, top).offsetParent parent.appendChild(this.container) if (this.options.fixed) { this.setPosition({ left, top }) From 3c0d2b79ef6f1b85dc07c355743ed4d363051c9b Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Sat, 5 Oct 2024 11:26:05 +0200 Subject: [PATCH 7/8] fix: allow to call contextmenu with given positions --- umap/static/umap/js/modules/ui/contextmenu.js | 5 +++++ umap/static/umap/js/umap.controls.js | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/umap/static/umap/js/modules/ui/contextmenu.js b/umap/static/umap/js/modules/ui/contextmenu.js index e85c562b..0787d5ea 100644 --- a/umap/static/umap/js/modules/ui/contextmenu.js +++ b/umap/static/umap/js/modules/ui/contextmenu.js @@ -15,9 +15,14 @@ export default class ContextMenu extends Positioned { }) } + open(event, items) { const left = event.clientX const top = event.clientY + this.openAt([left, top], items) + } + + openAt([left, top], items) { this.container.innerHTML = '' for (const item of items) { if (item === '-') { diff --git a/umap/static/umap/js/umap.controls.js b/umap/static/umap/js/umap.controls.js index 5bad389a..815ee1dc 100644 --- a/umap/static/umap/js/umap.controls.js +++ b/umap/static/umap/js/umap.controls.js @@ -663,7 +663,7 @@ const ControlsMixin = { button.addEventListener('click', () => { const x = button.offsetLeft const y = button.offsetTop + button.offsetHeight - menu.open([x, y], actions) + menu.openAt([x, y], actions) }) } this.help.getStartedLink(rightContainer) From 581ec242b8ba1dff7fc79ec29d45f5bb9aa960d1 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Sun, 6 Oct 2024 09:46:23 +0200 Subject: [PATCH 8/8] chore: always insert contextmenu on map container parent offsetParent will return whatever positionned parent element, so when clicking on a marker it will be the marker container. And we cannot add the contextmenu inside the map container, as it the focusout element will be sent before the click action happen on any item of the contextmenu. --- umap/static/umap/js/modules/ui/contextmenu.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/umap/static/umap/js/modules/ui/contextmenu.js b/umap/static/umap/js/modules/ui/contextmenu.js index 0787d5ea..8d2603b5 100644 --- a/umap/static/umap/js/modules/ui/contextmenu.js +++ b/umap/static/umap/js/modules/ui/contextmenu.js @@ -15,7 +15,6 @@ export default class ContextMenu extends Positioned { }) } - open(event, items) { const left = event.clientX const top = event.clientY @@ -43,7 +42,11 @@ export default class ContextMenu extends Positioned { this.container.appendChild(li) } } - const parent = document.elementFromPoint(left, top).offsetParent + // When adding contextmenu below the map container, clicking on any link will send the + // "focusout" element on link click, preventing to trigger the click action + const parent = document + .elementFromPoint(left, top) + .closest('.leaflet-container').parentNode parent.appendChild(this.container) if (this.options.fixed) { this.setPosition({ left, top })