From 48f9afdeddd95db69875b784322d9c54504ce224 Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Thu, 23 Jan 2025 11:25:09 +0100 Subject: [PATCH] feat: move star button to caption fix #2282 --- umap/static/umap/css/icon.css | 6 ++++++ umap/static/umap/img/16-white.svg | 2 ++ umap/static/umap/img/16.svg | 2 +- umap/static/umap/img/source/16-white.svg | 4 +++- umap/static/umap/img/source/16.svg | 2 +- umap/static/umap/js/modules/caption.js | 21 ++++++++++++++----- umap/static/umap/js/modules/rendering/map.js | 2 -- umap/static/umap/js/modules/schema.js | 6 ------ umap/static/umap/js/modules/umap.js | 7 ++++++- umap/static/umap/js/umap.controls.js | 12 ----------- umap/static/umap/map.css | 6 ------ umap/tests/integration/test_star.py | 22 ++++++++++++-------- umap/views.py | 11 +++++++++- 13 files changed, 58 insertions(+), 45 deletions(-) diff --git a/umap/static/umap/css/icon.css b/umap/static/umap/css/icon.css index 96bd85e9..df203121 100644 --- a/umap/static/umap/css/icon.css +++ b/umap/static/umap/css/icon.css @@ -153,6 +153,12 @@ html[dir="rtl"] .icon { .icon-share { background-position: 0px calc(var(--tile) * 5); } +.icon-star { + background-position: var(--tile) calc(var(--tile) * 7); +} +.icon-starred { + background-position: 0 calc(var(--tile) * 7); +} .icon-table { background-position: calc(var(--tile) * 2) 0px; } diff --git a/umap/static/umap/img/16-white.svg b/umap/static/umap/img/16-white.svg index a9f1484e..f174deb7 100644 --- a/umap/static/umap/img/16-white.svg +++ b/umap/static/umap/img/16-white.svg @@ -208,5 +208,7 @@ + + diff --git a/umap/static/umap/img/16.svg b/umap/static/umap/img/16.svg index 12221ede..d61b04b9 100644 --- a/umap/static/umap/img/16.svg +++ b/umap/static/umap/img/16.svg @@ -1 +1 @@ -image/svg+xml   +image/svg+xml   diff --git a/umap/static/umap/img/source/16-white.svg b/umap/static/umap/img/source/16-white.svg index e0e15e3c..3ed94e15 100644 --- a/umap/static/umap/img/source/16-white.svg +++ b/umap/static/umap/img/source/16-white.svg @@ -19,7 +19,7 @@ - + @@ -219,5 +219,7 @@ + + diff --git a/umap/static/umap/img/source/16.svg b/umap/static/umap/img/source/16.svg index 9cdf5ab3..8d69b43e 100644 --- a/umap/static/umap/img/source/16.svg +++ b/umap/static/umap/img/source/16.svg @@ -1,4 +1,4 @@ -image/svg+xml   +image/svg+xml   diff --git a/umap/static/umap/js/modules/caption.js b/umap/static/umap/js/modules/caption.js index e44c2e50..ecda2cc0 100644 --- a/umap/static/umap/js/modules/caption.js +++ b/umap/static/umap/js/modules/caption.js @@ -1,5 +1,6 @@ import { translate } from './i18n.js' import * as Utils from './utils.js' +import { uMapAlert as Alert } from '../components/alerts/alert.js' const TEMPLATE = `
@@ -7,8 +8,9 @@ const TEMPLATE = `

-

+

+
@@ -35,6 +37,14 @@ export default class Caption extends Utils.WithTemplate { this._umap = umap this._leafletMap = leafletMap this.loadTemplate(TEMPLATE) + this.elements.star.addEventListener('click', async () => { + if (this._umap.properties.user?.id) { + await this._umap.star() + this.refresh() + } else { + Alert.error(translate('You must be logged in')) + } + }) } isOpen() { @@ -62,10 +72,6 @@ export default class Caption extends Utils.WithTemplate { this.addDataLayer(datalayer, this.elements.datalayersContainer) ) this.addCredits() - this._umap.panel.open({ content: this.element }).then(() => { - // Create the legend when the panel is actually on the DOM - this._umap.eachDataLayerReverse((datalayer) => datalayer.renderLegend()) - }) if (this._umap.properties.created_at) { const created_at = translate('Created at {date}', { date: new Date(this._umap.properties.created_at).toLocaleDateString(), @@ -77,6 +83,11 @@ export default class Caption extends Utils.WithTemplate { } else { this.elements.dates.hidden = true } + this._umap.panel.open({ content: this.element }).then(() => { + // Create the legend when the panel is actually on the DOM + this._umap.eachDataLayerReverse((datalayer) => datalayer.renderLegend()) + this._umap.propagate() + }) } addDataLayer(datalayer, parent) { diff --git a/umap/static/umap/js/modules/rendering/map.js b/umap/static/umap/js/modules/rendering/map.js index b969c5ed..0b52454e 100644 --- a/umap/static/umap/js/modules/rendering/map.js +++ b/umap/static/umap/js/modules/rendering/map.js @@ -32,7 +32,6 @@ const ControlsMixin = { 'locate', 'measure', 'editinosm', - 'star', 'tilelayers', ], @@ -84,7 +83,6 @@ const ControlsMixin = { this._controls.search = new U.SearchControl() this._controls.embed = new Control.Embed(this._umap) this._controls.tilelayersChooser = new U.TileLayerChooser(this) - if (this.options.user?.id) this._controls.star = new U.StarControl(this._umap) this._controls.editinosm = new Control.EditInOSM({ position: 'topleft', widgetOptions: { diff --git a/umap/static/umap/js/modules/schema.js b/umap/static/umap/js/modules/schema.js index 54322c10..99a8645a 100644 --- a/umap/static/umap/js/modules/schema.js +++ b/umap/static/umap/js/modules/schema.js @@ -478,12 +478,6 @@ export const SCHEMA = { label: translate('Sort key'), inheritable: true, }, - starControl: { - type: Boolean, - impacts: ['ui'], - nullable: true, - label: translate('Display the star map button'), - }, stroke: { type: Boolean, impacts: ['data'], diff --git a/umap/static/umap/js/modules/umap.js b/umap/static/umap/js/modules/umap.js index ab4d94fe..f7205b35 100644 --- a/umap/static/umap/js/modules/umap.js +++ b/umap/static/umap/js/modules/umap.js @@ -1360,7 +1360,11 @@ export default class Umap extends ServerStored { }, 'properties.starred': () => { Utils.eachElement('.map-star', (el) => { - el.classList.toggle('starred', this.properties.starred) + el.classList.toggle('icon-starred', this.properties.starred) + el.classList.toggle('icon-star', !this.properties.starred) + }) + Utils.eachElement('.map-stars', (el) => { + el.textContent = this.properties.stars || translate('Star this map') }) }, } @@ -1543,6 +1547,7 @@ export default class Umap extends ServerStored { return } this.properties.starred = data.starred + this.properties.stars = data.stars Alert.success( data.starred ? translate('Map has been starred') diff --git a/umap/static/umap/js/umap.controls.js b/umap/static/umap/js/umap.controls.js index b0421ffe..61c43413 100644 --- a/umap/static/umap/js/umap.controls.js +++ b/umap/static/umap/js/umap.controls.js @@ -491,18 +491,6 @@ U.CaptionControl = L.Control.Button.extend({ }, }) -U.StarControl = L.Control.Button.extend({ - options: { - position: 'topleft', - title: L._('Star this map'), - className: 'leaflet-control-star map-star umap-control', - }, - - onClick: function () { - this._umap.star() - }, -}) - L.Control.Embed = L.Control.Button.extend({ options: { position: 'topleft', diff --git a/umap/static/umap/map.css b/umap/static/umap/map.css index 838faa33..aeaa125f 100644 --- a/umap/static/umap/map.css +++ b/umap/static/umap/map.css @@ -134,12 +134,6 @@ html[dir="rtl"] .leaflet-tooltip-pane > * { background-position: -72px -144px; box-shadow: 0 0 4px 0 black inset; } -.leaflet-control-star [type="button"] { - background-position: -144px -144px; -} -.leaflet-control-star.starred [type="button"] { - background-position: -108px -144px; -} .leaflet-control-search [type="button"] { background-position: -36px -108px; display: block; diff --git a/umap/tests/integration/test_star.py b/umap/tests/integration/test_star.py index 3984fb91..8e710e64 100644 --- a/umap/tests/integration/test_star.py +++ b/umap/tests/integration/test_star.py @@ -8,20 +8,24 @@ from umap.models import Star pytestmark = pytest.mark.django_db -def test_star_control_is_visible_if_logged_in(map, live_server, page, login, user): +def test_star_button_is_active_if_logged_in(map, live_server, page, login, user): login(user) assert not Star.objects.count() page.goto(f"{live_server.url}{map.get_absolute_url()}") - page.get_by_title("More controls").click() - control = page.locator(".leaflet-control-star") - expect(control).to_be_visible() + page.get_by_title("About").click() + button = page.locator(".icon-star") + expect(button).to_be_visible() with page.expect_response(re.compile(".*/star/")): - control.click() + button.click() + expect(button).to_be_hidden() + # Button has changed + expect(page.locator(".icon-starred")).to_be_visible() assert Star.objects.count() == 1 -def test_no_star_control_if_not_logged_in(map, live_server, page): +def test_star_button_inctive_if_not_logged_in(map, live_server, page): page.goto(f"{live_server.url}{map.get_absolute_url()}") - page.get_by_title("More controls").click() - control = page.locator(".leaflet-control-star") - expect(control).to_be_hidden() + page.get_by_title("About").click() + button = page.locator(".icon-star") + button.click() + expect(page.get_by_text("You must be logged in")).to_be_visible() diff --git a/umap/views.py b/umap/views.py index d1952405..9d865df3 100644 --- a/umap/views.py +++ b/umap/views.py @@ -605,6 +605,7 @@ class MapDetailMixin(SessionMixin): "schema": Map.extra_schema, "id": self.get_id(), "starred": self.is_starred(), + "stars": self.stars(), "licences": dict((l.name, l.json) for l in Licence.objects.all()), "umap_version": VERSION, "featuresHaveOwner": settings.UMAP_DEFAULT_FEATURES_HAVE_OWNERS, @@ -678,6 +679,9 @@ class MapDetailMixin(SessionMixin): def is_starred(self): return False + def stars(self): + return 0 + def get_geojson(self): return { "geometry": { @@ -780,6 +784,9 @@ class MapView(MapDetailMixin, PermissionsMixin, DetailView): return False return Star.objects.filter(by=user, map=self.object).exists() + def stars(self): + return Star.objects.filter(map=self.object).count() + class MapDownload(DetailView): model = Map @@ -1081,7 +1088,9 @@ class ToggleMapStarStatus(View): else: Star.objects.create(map=map_inst, by=self.request.user) status = True - return simple_json_response(starred=status) + return simple_json_response( + starred=status, stars=Star.objects.filter(map=map_inst).count() + ) class MapShortUrl(RedirectView):