From 762202c4b0b604c6e2d19b641fc10beb502641d7 Mon Sep 17 00:00:00 2001
From: Hugh Rundle <hugh@hughrundle.net>
Date: Mon, 27 Sep 2021 11:03:41 +1000
Subject: [PATCH] fix UI for group curated list editing

When creating or editing a list, the group selection dropdown will only appear if the user selects "group" as the curation option (or it is already selected).

- fix typo in bookwyrm.js comments
- add data-hides trigger for hiding elements after they have been unhidden, where simple toggles are not the right approach
---
 bookwyrm/static/js/bookwyrm.js     | 30 ++++++++++++++---
 bookwyrm/templates/lists/form.html | 52 +++++++++++++++---------------
 2 files changed, 52 insertions(+), 30 deletions(-)

diff --git a/bookwyrm/static/js/bookwyrm.js b/bookwyrm/static/js/bookwyrm.js
index f000fd082..049de497c 100644
--- a/bookwyrm/static/js/bookwyrm.js
+++ b/bookwyrm/static/js/bookwyrm.js
@@ -28,6 +28,12 @@ let BookWyrm = new class {
                 this.revealForm.bind(this))
             );
 
+        document.querySelectorAll('[data-hides]')
+            .forEach(button => button.addEventListener(
+                'change',
+                this.hideForm.bind(this))
+            );
+
         document.querySelectorAll('[data-back]')
             .forEach(button => button.addEventListener(
                 'click',
@@ -119,7 +125,7 @@ let BookWyrm = new class {
     }
 
     /**
-     * Toggle form.
+     * Show form.
      *
      * @param  {Event} event
      * @return {undefined}
@@ -127,10 +133,26 @@ let BookWyrm = new class {
     revealForm(event) {
         let trigger = event.currentTarget;
         let hidden = trigger.closest('.hidden-form').querySelectorAll('.is-hidden')[0];
-
-        this.addRemoveClass(hidden, 'is-hidden', !hidden);
+        // if the form has already been revealed, there is no '.is-hidden' element
+        // so this doesn't really work as a toggle
+        if (hidden) {
+          this.addRemoveClass(hidden, 'is-hidden', !hidden);
+        }
     }
 
+    /**
+     * Hide form.
+     *
+     * @param  {Event} event
+     * @return {undefined}
+     */
+    hideForm(event) {
+        let trigger = event.currentTarget;
+        let targetId = trigger.dataset.hides
+        let visible = document.getElementById(targetId)
+        this.addRemoveClass(visible, 'is-hidden', true);
+  }
+
     /**
      * Execute actions on targets based on triggers.
      *
@@ -227,7 +249,7 @@ let BookWyrm = new class {
     }
 
     /**
-     * Check or uncheck a checbox.
+     * Check or uncheck a checkbox.
      *
      * @param  {string}  checkbox - id of the checkbox
      * @param  {boolean} pressed  - Is the trigger pressed?
diff --git a/bookwyrm/templates/lists/form.html b/bookwyrm/templates/lists/form.html
index d2f17d63b..a98cae94a 100644
--- a/bookwyrm/templates/lists/form.html
+++ b/bookwyrm/templates/lists/form.html
@@ -18,45 +18,45 @@
         <fieldset class="field">
             <legend class="label">{% trans "List curation:" %}</legend>
 
-            <label class="field">
+            <label class="field" data-hides="list_group_selector">
                 <input type="radio" name="curation" value="closed"{% if not list or list.curation == 'closed' %} checked{% endif %}> {% trans "Closed" %}
                 <p class="help mb-2">{% trans "Only you can add and remove books to this list" %}</p>
             </label>
 
-            <label class="field">
+            <label class="field" data-hides="list_group_selector">
                 <input type="radio" name="curation" value="curated"{% if list.curation == 'curated' %} checked{% endif %}> {% trans "Curated" %}
                 <p class="help mb-2">{% trans "Anyone can suggest books, subject to your approval" %}</p>
             </label>
 
-            <label class="field">
+            <label class="field" data-hides="list_group_selector">
                 <input type="radio" name="curation" value="open"{% if list.curation == 'open' %} checked{% endif %}> {% trans "Open" %}
                 <p class="help mb-2">{% trans "Anyone can add books to this list" %}</p>
             </label>
 
-            <label class="field">
-              <input type="radio" name="curation" value="group"{% if list.curation == 'group' %} checked{% endif %}> {% trans "Group" %}
-              <p class="help mb-2">{% trans "Group members can add to and remove from this list" %}</p>
-          </label>
-          <!-- TODO: add a list picker on-select for group lists -->
-            {% if user_groups %}
-              {% csrf_token %}
-              <label class="label" for="id_group">{% trans "Select Group" %}</label>
-              <div class="field has-addons">
-                  <div class="select control">
-                      <select name="group" id="id_group">
-                          <option value="">Select a list</option>
-                          {% for group in user_groups %}
-                          <option value="{{ group.id }}" {% if list.group.id == group.id %} selected{% endif %}>{{ group.name }}</option>
-                          {% endfor %}
-                      </select>
-                  </div>
-              </div>
-              {% else %}
-              {% with user|username as username %}
-              {% url 'user-groups' user|username as url %}
-              <div>{% trans "You must create a " %}<a href="{{ url }}">{% trans "Group" %}</a>{% trans " before you can create Group lists!" %}</div>
-              {% endwith %}
+            <label class="field hidden-form">
+                <input type="radio" name="curation" value="group"{% if list.curation == 'group' %} checked{% endif %} > {% trans "Group" %}
+                <p class="help mb-2">{% trans "Group members can add to and remove from this list" %}</p>
+                <fieldset class="{% if list.curation != 'group' %}is-hidden{% endif %}" id="list_group_selector">
+                  {% if user_groups %}
+                <label class="label" for="id_group" id="group">{% trans "Select Group" %}</label>
+                <div class="field has-addons">
+                    <div class="select control">
+                        <select name="group" id="id_group">
+                            <option value="" disabled {% if not list.group %} selected{% endif %}>{% trans "Select a list" %}</option>
+                            {% for group in user_groups %}
+                            <option value="{{ group.id }}" {% if list.group.id == group.id %} selected{% endif %}>{{ group.name }}</option>
+                            {% endfor %}
+                        </select>
+                    </div>
+                </div>
+                {% else %}
+                {% with user|username as username %}
+                {% url 'user-groups' user|username as url %}
+                <div>{% trans "You must create a " %}<a href="{{ url }}">{% trans "Group" %}</a>{% trans " before you can create Group lists!" %}</div>
+                {% endwith %}
+              </fieldset>
               {% endif %}
+            </label>
         </fieldset>
     </div>
 </div>