From c860866fe9df5ff5d6ea502534f94ab26fee68b3 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Thu, 16 May 2024 19:38:51 +0200 Subject: [PATCH 1/2] feat: add defaultPanelMode setting cf https://forum.openstreetmap.fr/t/umap-2-3-changement-de-gestion-des-pop-up/23680/3 Trying to make this panel expanded/condenses thing simpler and more intuitive. It's mode can be set: - by explicitely setting defaultPanelMode = xxx - or, if defaultPanelMode is undefined, with sensible default when there is an onLoadPanel defined, and to respect previous uMap behaviour - or, if defaultPanelMode is unset, and some feature opens in the panel, it will be set to expanded (here again to respect previous behaviour - then, when user change it manually (by clicking on the toggle button), then we should never change it automatically, and respect the previous mode when reopening the panel (We are only talking about the left panel, here.) --- umap/static/umap/base.css | 1 + umap/static/umap/js/modules/browser.js | 13 +-------- umap/static/umap/js/modules/panel.js | 20 ++++++++----- umap/static/umap/js/modules/schema.js | 8 +++++ umap/static/umap/js/umap.forms.js | 6 +++- umap/static/umap/js/umap.js | 9 ++++-- umap/static/umap/js/umap.popup.js | 1 + umap/tests/integration/test_browser.py | 14 +++++++-- umap/tests/integration/test_caption.py | 16 ++++++++-- umap/tests/integration/test_facets_browser.py | 5 +++- umap/tests/integration/test_view_marker.py | 29 +++++++++++++++++++ 11 files changed, 93 insertions(+), 29 deletions(-) diff --git a/umap/static/umap/base.css b/umap/static/umap/base.css index f7c9969a..b71c252e 100644 --- a/umap/static/umap/base.css +++ b/umap/static/umap/base.css @@ -465,6 +465,7 @@ input.switch:checked ~ label:after { .button-bar { grid-gap: 7px; } +.umap-multiplechoice.by2, .button-bar.half { grid-template-columns: 1fr 1fr; } diff --git a/umap/static/umap/js/modules/browser.js b/umap/static/umap/js/modules/browser.js index 24700a6f..e57e0c22 100644 --- a/umap/static/umap/js/modules/browser.js +++ b/umap/static/umap/js/modules/browser.js @@ -9,18 +9,7 @@ export default class Browser { filter: '', inBbox: false, } - this._mode = 'layers' - } - - set mode(value) { - // Store the mode so we can respect it when we redraw - if (['data', 'filters'].includes(value)) this.map.panel.mode = 'expanded' - else if (value === 'layers') this.map.panel.mode = 'condensed' - this._mode = value - } - - get mode() { - return this._mode + this.mode = 'layers' } addFeature(feature, parent) { diff --git a/umap/static/umap/js/modules/panel.js b/umap/static/umap/js/modules/panel.js index 6bbc5262..62041f6b 100644 --- a/umap/static/umap/js/modules/panel.js +++ b/umap/static/umap/js/modules/panel.js @@ -2,11 +2,11 @@ import { DomUtil, DomEvent } from '../../vendors/leaflet/leaflet-src.esm.js' import { translate } from './i18n.js' export class Panel { - constructor(map) { + constructor(map, mode = null) { this.parent = map._controlContainer this.map = map this.container = DomUtil.create('div', '', this.parent) - this.mode = 'condensed' + this.mode = mode this.classname = 'left' DomEvent.disableClickPropagation(this.container) DomEvent.on(this.container, 'contextmenu', DomEvent.stopPropagation) // Do not activate our custom context menu. @@ -14,8 +14,12 @@ export class Panel { DomEvent.on(this.container, 'MozMousePixelScroll', DomEvent.stopPropagation) } + setDefaultMode(mode) { + if (!this.mode) this.mode = mode + } + open({ content, className, actions = [] } = {}) { - this.container.className = `with-transition panel ${this.classname} ${this.mode}` + this.container.className = `with-transition panel ${this.classname} ${this.mode || ''}` this.container.innerHTML = '' const actionsContainer = DomUtil.create('ul', 'toolbox', this.container) const body = DomUtil.create('div', 'body', this.container) @@ -40,14 +44,14 @@ export class Panel { } resize() { - if (this.mode === 'expanded') { - this.mode = 'condensed' - this.container.classList.remove('expanded') - this.container.classList.add('condensed') - } else { + if (this.mode === 'condensed') { this.mode = 'expanded' this.container.classList.remove('condensed') this.container.classList.add('expanded') + } else { + this.mode = 'condensed' + this.container.classList.remove('expanded') + this.container.classList.add('condensed') } } diff --git a/umap/static/umap/js/modules/schema.js b/umap/static/umap/js/modules/schema.js index 3db6f726..632a226d 100644 --- a/umap/static/umap/js/modules/schema.js +++ b/umap/static/umap/js/modules/schema.js @@ -52,6 +52,14 @@ export const SCHEMA = { label: translate('Display the data layers control'), default: true, }, + defaultPanelMode: { + type: String, + label: translate('Default panel mode'), + choices: [ + ['condensed', translate('Condensed')], + ['expanded', translate('Expanded')], + ], + }, defaultView: { type: String, impacts: [], // no need to update the ui, only useful when loading the map diff --git a/umap/static/umap/js/umap.forms.js b/umap/static/umap/js/umap.forms.js index bb38aebe..c52ad146 100644 --- a/umap/static/umap/js/umap.forms.js +++ b/umap/static/umap/js/umap.forms.js @@ -941,7 +941,11 @@ L.FormBuilder.MultiChoice = L.FormBuilder.Element.extend({ if (!this.container.querySelector(`input[type="radio"][value="${value}"]`)) { value = this.options.default !== undefined ? this.options.default : this.default } - this.container.querySelector(`input[type="radio"][value="${value}"]`).checked = true + const choices = this.getChoices().map(([value, label]) => value) + if (choices.includes(value)) { + this.container.querySelector(`input[type="radio"][value="${value}"]`).checked = + true + } }, value: function () { diff --git a/umap/static/umap/js/umap.js b/umap/static/umap/js/umap.js index 8bc9ac21..851b9959 100644 --- a/umap/static/umap/js/umap.js +++ b/umap/static/umap/js/umap.js @@ -56,7 +56,7 @@ U.Map = L.Map.extend({ if (geojson.geometry) this.options.center = this.latLng(geojson.geometry) this.urls = new U.URLs(this.options.urls) - this.panel = new U.Panel(this) + this.panel = new U.Panel(this, this.options.defaultPanelMode) if (this.hasEditMode()) { this.editPanel = new U.EditPanel(this) this.fullPanel = new U.FullPanel(this) @@ -214,14 +214,16 @@ U.Map = L.Map.extend({ if (L.Util.queryString('share')) { this.share.open() } else if (this.options.onLoadPanel === 'databrowser') { + this.panel.setDefaultMode('expanded') this.openBrowser('data') } else if (this.options.onLoadPanel === 'datalayers') { + this.panel.setDefaultMode('condensed') this.openBrowser('layers') } else if (this.options.onLoadPanel === 'datafilters') { - this.panel.mode = 'expanded' + this.panel.setDefaultMode('expanded') this.openBrowser('filters') } else if (this.options.onLoadPanel === 'caption') { - this.panel.mode = 'condensed' + this.panel.setDefaultMode('condensed') this.openCaption() } if (L.Util.queryString('edit')) { @@ -1172,6 +1174,7 @@ U.Map = L.Map.extend({ 'options.scaleControl', 'options.onLoadPanel', 'options.defaultView', + 'options.defaultPanelMode', 'options.displayPopupFooter', 'options.captionBar', 'options.captionMenus', diff --git a/umap/static/umap/js/umap.popup.js b/umap/static/umap/js/umap.popup.js index b81576a3..69c01b13 100644 --- a/umap/static/umap/js/umap.popup.js +++ b/umap/static/umap/js/umap.popup.js @@ -55,6 +55,7 @@ U.Popup.Panel = U.Popup.extend({ }, onAdd: function (map) { + map.panel.setDefaultMode('expanded') map.panel.open({ content: this._content, actions: [U.Browser.backButton(map)], diff --git a/umap/tests/integration/test_browser.py b/umap/tests/integration/test_browser.py index 90e3be9c..ea1d7a6a 100644 --- a/umap/tests/integration/test_browser.py +++ b/umap/tests/integration/test_browser.py @@ -1,3 +1,4 @@ +import re from copy import deepcopy from time import sleep @@ -63,13 +64,22 @@ def bootstrap(map, live_server): def test_data_browser_should_be_open(live_server, page, bootstrap, map): page.goto(f"{live_server.url}{map.get_absolute_url()}") - el = page.locator(".umap-browser") - expect(el).to_be_visible() + panel = page.locator(".panel.left.on") + expect(panel).to_have_class(re.compile(".*expanded.*")) + expect(panel.locator(".umap-browser")).to_be_visible() expect(page.get_by_text("one point in france")).to_be_visible() expect(page.get_by_text("one line in new zeland")).to_be_visible() expect(page.get_by_text("one polygon in greenland")).to_be_visible() +def test_can_force_panel_mode(live_server, page, bootstrap, map): + map.settings["properties"]["defaultPanelMode"] = "condensed" + map.save() + page.goto(f"{live_server.url}{map.get_absolute_url()}") + panel = page.locator(".panel.left.on") + expect(panel).to_have_class(re.compile(".*condensed.*")) + + def test_data_browser_should_be_filterable(live_server, page, bootstrap, map): page.goto(f"{live_server.url}{map.get_absolute_url()}") expect(page.get_by_title("Features in this layer: 3")).to_be_visible() diff --git a/umap/tests/integration/test_caption.py b/umap/tests/integration/test_caption.py index 2936d65c..33083c40 100644 --- a/umap/tests/integration/test_caption.py +++ b/umap/tests/integration/test_caption.py @@ -1,3 +1,5 @@ +import re + import pytest from playwright.sync_api import expect @@ -15,10 +17,20 @@ def test_caption(live_server, page, map): ) hidden = DataLayerFactory(map=map, name="Hidden", settings={"inCaption": False}) page.goto(f"{live_server.url}{map.get_absolute_url()}") - panel = page.locator(".umap-caption") - expect(panel).to_be_visible() + panel = page.locator(".panel.left.on") + expect(panel).to_have_class(re.compile(".*condensed.*")) + expect(panel.locator(".umap-caption")).to_be_visible() expect(panel.locator(".datalayer-legend").get_by_text(basic.name)).to_be_visible() expect( panel.locator(".datalayer-legend .off").get_by_text(non_loaded.name) ).to_be_visible() expect(panel.locator(".datalayer-legend").get_by_text(hidden.name)).to_be_hidden() + + +def test_can_force_panel_mode(live_server, page, map): + map.settings["properties"]["onLoadPanel"] = "caption" + map.settings["properties"]["defaultPanelMode"] = "expanded" + map.save() + page.goto(f"{live_server.url}{map.get_absolute_url()}") + panel = page.locator(".panel.left.on") + expect(panel).to_have_class(re.compile(".*expanded.*")) diff --git a/umap/tests/integration/test_facets_browser.py b/umap/tests/integration/test_facets_browser.py index 0ba5a621..c386cc3c 100644 --- a/umap/tests/integration/test_facets_browser.py +++ b/umap/tests/integration/test_facets_browser.py @@ -1,4 +1,5 @@ import copy +import re import pytest from playwright.sync_api import expect @@ -101,7 +102,9 @@ def test_simple_facet_search(live_server, page, map): DataLayerFactory(map=map, data=DATALAYER_DATA2) DataLayerFactory(map=map, data=DATALAYER_DATA3) page.goto(f"{live_server.url}{map.get_absolute_url()}#6/48.948/1.670") - panel = page.locator(".umap-browser") + panel = page.locator(".panel.left.on") + expect(panel).to_have_class(re.compile(".*expanded.*")) + expect(panel.locator(".umap-browser")).to_be_visible() # From a non browsable datalayer, should not be impacted paths = page.locator(".leaflet-overlay-pane path") expect(paths).to_be_visible() diff --git a/umap/tests/integration/test_view_marker.py b/umap/tests/integration/test_view_marker.py index c0c5002a..4602cffd 100644 --- a/umap/tests/integration/test_view_marker.py +++ b/umap/tests/integration/test_view_marker.py @@ -1,3 +1,4 @@ +import re from copy import deepcopy import pytest @@ -62,3 +63,31 @@ def test_should_display_tooltip_with_variable(live_server, map, page, bootstrap) map.save() page.goto(f"{live_server.url}{map.get_absolute_url()}") expect(page.get_by_text("Foo test marker")).to_be_visible() + + +def test_should_open_popup_panel_on_click(live_server, map, page, bootstrap): + map.settings["properties"]["popupShape"] = "Panel" + map.save() + page.goto(f"{live_server.url}{map.get_absolute_url()}") + panel = page.locator(".panel.left.on") + expect(panel).to_be_hidden() + page.locator(".leaflet-marker-icon").click() + expect(panel).to_be_visible() + expect(panel).to_have_class(re.compile(".*expanded.*")) + expect(panel.get_by_role("heading", name="test marker")).to_be_visible() + expect(panel.get_by_text("Some description")).to_be_visible() + # Close popup + page.locator("#map").click() + expect(panel).to_be_hidden() + + +def test_can_force_popup_panel_mode(live_server, map, page, bootstrap): + map.settings["properties"]["popupShape"] = "Panel" + map.settings["properties"]["defaultPanelMode"] = "condensed" + map.save() + page.goto(f"{live_server.url}{map.get_absolute_url()}") + panel = page.locator(".panel.left.on") + expect(panel).to_be_hidden() + page.locator(".leaflet-marker-icon").click() + expect(panel).to_be_visible() + expect(panel).to_have_class(re.compile(".*condensed.*")) From 284ef1d7f31ac5bed512039745575aa4ca895cbf Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Fri, 17 May 2024 17:13:37 +0200 Subject: [PATCH 2/2] feat: remove defaultPanelMode from now After discussion, we prefer to go with decent defaults and see how it goes. --- umap/static/umap/js/modules/panel.js | 6 ++++-- umap/static/umap/js/modules/schema.js | 8 -------- umap/static/umap/js/umap.js | 3 +-- umap/tests/integration/test_browser.py | 8 -------- umap/tests/integration/test_caption.py | 9 --------- umap/tests/integration/test_view_marker.py | 12 ------------ 6 files changed, 5 insertions(+), 41 deletions(-) diff --git a/umap/static/umap/js/modules/panel.js b/umap/static/umap/js/modules/panel.js index 62041f6b..48c17e41 100644 --- a/umap/static/umap/js/modules/panel.js +++ b/umap/static/umap/js/modules/panel.js @@ -2,11 +2,13 @@ import { DomUtil, DomEvent } from '../../vendors/leaflet/leaflet-src.esm.js' import { translate } from './i18n.js' export class Panel { - constructor(map, mode = null) { + constructor(map) { this.parent = map._controlContainer this.map = map this.container = DomUtil.create('div', '', this.parent) - this.mode = mode + // This will be set once according to the panel configurated at load + // or by using panels as popups + this.mode = null this.classname = 'left' DomEvent.disableClickPropagation(this.container) DomEvent.on(this.container, 'contextmenu', DomEvent.stopPropagation) // Do not activate our custom context menu. diff --git a/umap/static/umap/js/modules/schema.js b/umap/static/umap/js/modules/schema.js index 632a226d..3db6f726 100644 --- a/umap/static/umap/js/modules/schema.js +++ b/umap/static/umap/js/modules/schema.js @@ -52,14 +52,6 @@ export const SCHEMA = { label: translate('Display the data layers control'), default: true, }, - defaultPanelMode: { - type: String, - label: translate('Default panel mode'), - choices: [ - ['condensed', translate('Condensed')], - ['expanded', translate('Expanded')], - ], - }, defaultView: { type: String, impacts: [], // no need to update the ui, only useful when loading the map diff --git a/umap/static/umap/js/umap.js b/umap/static/umap/js/umap.js index 851b9959..0fd4fd63 100644 --- a/umap/static/umap/js/umap.js +++ b/umap/static/umap/js/umap.js @@ -56,7 +56,7 @@ U.Map = L.Map.extend({ if (geojson.geometry) this.options.center = this.latLng(geojson.geometry) this.urls = new U.URLs(this.options.urls) - this.panel = new U.Panel(this, this.options.defaultPanelMode) + this.panel = new U.Panel(this) if (this.hasEditMode()) { this.editPanel = new U.EditPanel(this) this.fullPanel = new U.FullPanel(this) @@ -1174,7 +1174,6 @@ U.Map = L.Map.extend({ 'options.scaleControl', 'options.onLoadPanel', 'options.defaultView', - 'options.defaultPanelMode', 'options.displayPopupFooter', 'options.captionBar', 'options.captionMenus', diff --git a/umap/tests/integration/test_browser.py b/umap/tests/integration/test_browser.py index ea1d7a6a..29f35290 100644 --- a/umap/tests/integration/test_browser.py +++ b/umap/tests/integration/test_browser.py @@ -72,14 +72,6 @@ def test_data_browser_should_be_open(live_server, page, bootstrap, map): expect(page.get_by_text("one polygon in greenland")).to_be_visible() -def test_can_force_panel_mode(live_server, page, bootstrap, map): - map.settings["properties"]["defaultPanelMode"] = "condensed" - map.save() - page.goto(f"{live_server.url}{map.get_absolute_url()}") - panel = page.locator(".panel.left.on") - expect(panel).to_have_class(re.compile(".*condensed.*")) - - def test_data_browser_should_be_filterable(live_server, page, bootstrap, map): page.goto(f"{live_server.url}{map.get_absolute_url()}") expect(page.get_by_title("Features in this layer: 3")).to_be_visible() diff --git a/umap/tests/integration/test_caption.py b/umap/tests/integration/test_caption.py index 33083c40..1458a8a3 100644 --- a/umap/tests/integration/test_caption.py +++ b/umap/tests/integration/test_caption.py @@ -25,12 +25,3 @@ def test_caption(live_server, page, map): panel.locator(".datalayer-legend .off").get_by_text(non_loaded.name) ).to_be_visible() expect(panel.locator(".datalayer-legend").get_by_text(hidden.name)).to_be_hidden() - - -def test_can_force_panel_mode(live_server, page, map): - map.settings["properties"]["onLoadPanel"] = "caption" - map.settings["properties"]["defaultPanelMode"] = "expanded" - map.save() - page.goto(f"{live_server.url}{map.get_absolute_url()}") - panel = page.locator(".panel.left.on") - expect(panel).to_have_class(re.compile(".*expanded.*")) diff --git a/umap/tests/integration/test_view_marker.py b/umap/tests/integration/test_view_marker.py index 4602cffd..8dbe3009 100644 --- a/umap/tests/integration/test_view_marker.py +++ b/umap/tests/integration/test_view_marker.py @@ -79,15 +79,3 @@ def test_should_open_popup_panel_on_click(live_server, map, page, bootstrap): # Close popup page.locator("#map").click() expect(panel).to_be_hidden() - - -def test_can_force_popup_panel_mode(live_server, map, page, bootstrap): - map.settings["properties"]["popupShape"] = "Panel" - map.settings["properties"]["defaultPanelMode"] = "condensed" - map.save() - page.goto(f"{live_server.url}{map.get_absolute_url()}") - panel = page.locator(".panel.left.on") - expect(panel).to_be_hidden() - page.locator(".leaflet-marker-icon").click() - expect(panel).to_be_visible() - expect(panel).to_have_class(re.compile(".*condensed.*"))