From 63e84d94c4826e8c165476cc325c61f3b0f70fac Mon Sep 17 00:00:00 2001
From: Yohan Boniface <yohanboniface@free.fr>
Date: Wed, 8 Jan 2025 16:14:36 +0100
Subject: [PATCH] chore(forms): refactor icon preview of IconURL field

---
 umap/static/umap/css/form.css                 | 29 ++++++++-----------
 umap/static/umap/js/modules/form/builder.js   |  8 +++--
 umap/static/umap/js/modules/form/fields.js    | 24 +++++++--------
 umap/tests/integration/test_edit_datalayer.py |  2 +-
 umap/tests/integration/test_picto.py          | 16 +++++-----
 5 files changed, 37 insertions(+), 42 deletions(-)

diff --git a/umap/static/umap/css/form.css b/umap/static/umap/css/form.css
index fa92e9ad..e4ad4427 100644
--- a/umap/static/umap/css/form.css
+++ b/umap/static/umap/css/form.css
@@ -382,16 +382,19 @@ input.switch:checked ~ label:after {
     box-shadow: inset 0 0 6px 0px #2c3233;
     color: var(--color-darkGray);
 }
-.inheritable .header,
-.inheritable {
-    clear: both;
-    overflow: hidden;
+.inheritable .header .buttons {
+    padding: 0;
 }
 .inheritable .header {
     margin-bottom: 5px;
+    display: flex;
+    align-items: center;
+    align-content: center;
+    justify-content: space-between;
 }
 .inheritable .header label {
     padding-top: 6px;
+    width: initial;
 }
 .inheritable + .inheritable {
     border-top: 1px solid #222;
@@ -401,22 +404,11 @@ input.switch:checked ~ label:after {
 .umap-field-iconUrl .action-button,
 .inheritable .define,
 .inheritable .undefine {
-    float: inline-end;
     width: initial;
     min-height: 18px;
     line-height: 18px;
     margin-bottom: 0;
 }
-.inheritable .quick-actions {
-    float: inline-end;
-}
-.inheritable .quick-actions .formbox {
-    margin-bottom: 0;
-}
-.inheritable .quick-actions input {
-    width: 100px;
-    margin-inline-end: 5px;
-}
 .inheritable .define,
 .inheritable.undefined .undefine,
 .inheritable.undefined .show-on-defined {
@@ -494,12 +486,15 @@ i.info {
     padding: 0 5px;
 }
 .flat-tabs {
-    display: flex;
+    display: none;
     justify-content: space-around;
     font-size: 1.2em;
     margin-bottom: 20px;
     border-bottom: 1px solid #bebebe;
 }
+.flat-tabs:has(.flat) {
+    display: flex;
+}
 .flat-tabs button {
     padding: 10px;
     text-decoration: none;
@@ -535,7 +530,7 @@ i.info {
     background-color: #999;
     text-align: center;
     margin-bottom: 5px;
-    display: block;
+    display: inline-block;
     color: black;
     font-weight: bold;
 }
diff --git a/umap/static/umap/js/modules/form/builder.js b/umap/static/umap/js/modules/form/builder.js
index 5af0bfac..e16b9b7a 100644
--- a/umap/static/umap/js/modules/form/builder.js
+++ b/umap/static/umap/js/modules/form/builder.js
@@ -205,10 +205,12 @@ export class MutatingForm extends Form {
       template = `
         <div class="umap-field-${helper.name} formbox inheritable${extraClassName}">
           <div class="header" data-ref=header>
-            <a href="#" class="button undefine" data-ref=undefine>${translate('clear')}</a>
-            <a href="#" class="button define" data-ref=define>${translate('define')}</a>
-            <span class="quick-actions show-on-defined" data-ref=actions></span>
             ${helper.getLabelTemplate()}
+            <span class="actions show-on-defined" data-ref=actions></span>
+            <span class="buttons" data-ref=buttons>
+              <button type="button" class="button undefine" data-ref=undefine>${translate('clear')}</button>
+              <button type="button" class="button define" data-ref=define>${translate('define')}</button>
+            </span>
           </div>
           <div class="show-on-defined" data-ref=container>
             ${helper.getTemplate()}
diff --git a/umap/static/umap/js/modules/form/fields.js b/umap/static/umap/js/modules/form/fields.js
index fc4ac499..29fc03b7 100644
--- a/umap/static/umap/js/modules/form/fields.js
+++ b/umap/static/umap/js/modules/form/fields.js
@@ -664,7 +664,6 @@ Fields.IconUrl = class extends Fields.BlurInput {
   getTemplate() {
     return `
       <div>
-        <div data-ref=buttons></div>
         <div class="flat-tabs" data-ref=tabs></div>
         <div class="umap-pictogram-body" data-ref=body>
         ${super.getTemplate()}
@@ -676,15 +675,18 @@ Fields.IconUrl = class extends Fields.BlurInput {
 
   build() {
     super.build()
-    this.buttons = this.elements.buttons
     this.tabs = this.elements.tabs
     this.body = this.elements.body
     this.footer = this.elements.footer
+    this.button = Utils.loadTemplate(
+      `<button type="button" class="button action-button" hidden>${translate('Change')}</button>`
+    )
+    this.button.addEventListener('click', () => this.onDefine())
+    this.elements.buttons.appendChild(this.button)
     this.updatePreview()
   }
 
   async onDefine() {
-    this.buttons.innerHTML = ''
     this.footer.innerHTML = ''
     const [{ pictogram_list }, response, error] = await this.builder._umap.server.get(
       this.builder._umap.properties.urls.pictogram_list_json
@@ -692,14 +694,14 @@ Fields.IconUrl = class extends Fields.BlurInput {
     if (!error) this.pictogram_list = pictogram_list
     this.buildTabs()
     const value = this.value()
-    if (U.Icon.RECENT.length) this.showRecentTab()
+    if (Icon.RECENT.length) this.showRecentTab()
     else if (!value || Utils.isPath(value)) this.showSymbolsTab()
     else if (Utils.isRemoteUrl(value) || Utils.isDataImage(value)) this.showURLTab()
     else this.showCharsTab()
     const closeButton = Utils.loadTemplate(
       `<button type="button" class="button action-button">${translate('Close')}</button>`
     )
-    closeButton.addEventListener('click', () => {
+    closeButton.addEventListener('click', (event) => {
       this.body.innerHTML = ''
       this.tabs.innerHTML = ''
       this.footer.innerHTML = ''
@@ -758,21 +760,16 @@ Fields.IconUrl = class extends Fields.BlurInput {
   }
 
   updatePreview() {
-    this.buttons.innerHTML = ''
+    this.elements.actions.innerHTML = ''
+    this.button.hidden = !this.value() || this.isDefault()
     if (this.isDefault()) return
     if (!Utils.hasVar(this.value())) {
       // Do not try to render URL with variables
       const box = Utils.loadTemplate('<div class="umap-pictogram-choice"></div>')
-      this.buttons.appendChild(box)
+      this.elements.actions.appendChild(box)
       box.addEventListener('click', () => this.onDefine())
       const icon = Icon.makeElement(this.value(), box)
     }
-    const text = this.value() ? translate('Change') : translate('Add')
-    const button = Utils.loadTemplate(
-      `<button type="button" class="button action-button">${text}</button>`
-    )
-    button.addEventListener('click', () => this.onDefine())
-    this.buttons.appendChild(button)
   }
 
   addIconPreview(pictogram, parent) {
@@ -794,6 +791,7 @@ Fields.IconUrl = class extends Fields.BlurInput {
       this.sync()
       this.unselectAll(this.grid)
       container.classList.add('selected')
+      this.updatePreview()
     })
     return true // Icon has been added (not filtered)
   }
diff --git a/umap/tests/integration/test_edit_datalayer.py b/umap/tests/integration/test_edit_datalayer.py
index f9a9fa07..a14c8764 100644
--- a/umap/tests/integration/test_edit_datalayer.py
+++ b/umap/tests/integration/test_edit_datalayer.py
@@ -103,7 +103,7 @@ def test_can_change_icon_class(live_server, openmap, page):
     expect(page.locator(".umap-circle-icon")).to_be_hidden()
     page.locator(".panel.right").get_by_title("Edit", exact=True).click()
     page.get_by_text("Shape properties").click()
-    page.locator(".umap-field-iconClass a.define").click()
+    page.locator(".umap-field-iconClass button.define").click()
     page.get_by_text("Circle", exact=True).click()
     expect(page.locator(".umap-circle-icon")).to_be_visible()
     expect(page.locator(".umap-div-icon")).to_be_hidden()
diff --git a/umap/tests/integration/test_picto.py b/umap/tests/integration/test_picto.py
index d4b38954..f561a459 100644
--- a/umap/tests/integration/test_picto.py
+++ b/umap/tests/integration/test_picto.py
@@ -57,7 +57,7 @@ def test_can_change_picto_at_map_level(openmap, live_server, page, pictos):
     define.click()
     # No picto defined yet, so recent should not be visible
     expect(page.get_by_text("Recent")).to_be_hidden()
-    symbols = page.locator(".umap-pictogram-choice")
+    symbols = page.locator(".umap-pictogram-body .umap-pictogram-choice")
     expect(symbols).to_have_count(2)
     search = page.locator(".umap-pictogram-body input")
     search.type("star")
@@ -90,8 +90,8 @@ def test_can_change_picto_at_datalayer_level(openmap, live_server, page, pictos)
     expect(define).to_be_visible()
     expect(undefine).to_be_hidden()
     define.click()
-    # Map has an icon defined, so it shold open on Recent tab
-    symbols = page.locator(".umap-pictogram-choice")
+    # Map has an icon defined, so it should open on Recent tab
+    symbols = page.locator(".umap-pictogram-body .umap-pictogram-choice")
     expect(page.get_by_text("Recent")).to_be_visible()
     expect(symbols).to_have_count(1)
     symbol_tab = page.get_by_role("button", name="Symbol")
@@ -128,8 +128,8 @@ def test_can_change_picto_at_marker_level(openmap, live_server, page, pictos):
     expect(define).to_be_visible()
     expect(undefine).to_be_hidden()
     define.click()
-    # Map has an icon defined, so it shold open on Recent tab
-    symbols = page.locator(".umap-pictogram-choice")
+    # Map has an icon defined, so it shuold open on Recent tab
+    symbols = page.locator(".umap-pictogram-body .umap-pictogram-choice")
     expect(page.get_by_text("Recent")).to_be_visible()
     expect(symbols).to_have_count(1)
     symbol_tab = page.get_by_role("button", name="Symbol")
@@ -180,7 +180,7 @@ def test_can_use_remote_url_as_picto(openmap, live_server, page, pictos):
     expect(modify).to_be_visible()
     modify.click()
     # Should be on Recent tab
-    symbols = page.locator(".umap-pictogram-choice")
+    symbols = page.locator(".umap-pictogram-body .umap-pictogram-choice")
     expect(page.get_by_text("Recent")).to_be_visible()
     expect(symbols).to_have_count(1)
 
@@ -215,10 +215,10 @@ def test_can_use_char_as_picto(openmap, live_server, page, pictos):
     close.click()
     edit_settings.click()
     shape_settings.click()
-    preview = page.locator(".umap-pictogram-choice")
+    preview = page.locator(".header .umap-pictogram-choice")
     expect(preview).to_be_visible()
     preview.click()
     # Should be on URL tab
-    symbols = page.locator(".umap-pictogram-choice")
+    symbols = page.locator(".umap-pictogram-body .umap-pictogram-choice")
     expect(page.get_by_text("Recent")).to_be_visible()
     expect(symbols).to_have_count(1)