Merge pull request #1471 from bookwyrm-social/goal-cleanup

Improves html consistency for goal page

bookwyrm/templates/components/inline_form.html

@@ -1,5 +1,5 @@
 {% load i18n %}
-<section class="card is-hidden {{ class }}" id="{{ controls_text }}{% if controls_uid %}-{{ controls_uid }}{% endif %}">
+<section class="card {% if not visible %}is-hidden {% endif %}{{ class }}" id="{{ controls_text }}{% if controls_uid %}-{{ controls_uid }}{% endif %}">
     <header class="card-header has-background-white-ter">
         <h2 class="card-header-title" tabindex="0" id="{{ controls_text }}{% if controls_uid %}-{{ controls_uid }}{% endif %}_header">
             {% block header %}{% endblock %}

bookwyrm/templates/feed/feed.html

@@ -25,7 +25,7 @@
 {% if request.user.show_goal and not goal and tab.key == 'home' %}
 {% now 'Y' as year %}
 <section class="block">
-    {% include 'snippets/goal_card.html' with year=year %}
+    {% include 'feed/goal_card.html' with year=year %}
     <hr>
 </section>
 {% endif %}

bookwyrm/templates/feed/goal_card.html

@@ -7,13 +7,8 @@
 </h3>
 {% endblock %}
 
-
 {% block card-content %}
-<div class="content">
-    <p>{% blocktrans %}Set a goal for how many books you'll finish reading in {{ year }}, and track your progress throughout the year.{% endblocktrans %}</p>
-
 {% include 'snippets/goal_form.html' %}
-</div>
 {% endblock %}
 
 {% block card-footer %}

bookwyrm/templates/snippets/goal_form.html

@@ -1,4 +1,7 @@
 {% load i18n %}
+<div class="content">
+    <p>{% blocktrans %}Set a goal for how many books you'll finish reading in {{ year }}, and track your progress throughout the year.{% endblocktrans %}</p>
+
     <form method="post" name="goal" action="{% url 'user-goal' request.user.localname year %}">
         {% csrf_token %}
         <input type="hidden" name="year" value="{% if goal %}{{ goal.year }}{% else %}{{ year }}{% endif %}">
@@ -16,21 +19,18 @@
             </div>
 
             <div class="column">
-            <label class="label"><p class="mb-2">{% trans "Goal privacy:" %}</p>
+                <label class="label" for="privacy_{{ goal.id }}">{% trans "Goal privacy:" %}</label>
-                {% include 'snippets/privacy_select.html' with no_label=True current=goal.privacy %}
+                {% include 'snippets/privacy_select.html' with no_label=True current=goal.privacy uuid=goal.id %}
-            </label>
             </div>
         </div>
+
         <label for="post_status" class="label">
             <input type="checkbox" name="post-status" id="post_status" class="checkbox" checked>
             {% trans "Post to feed" %}
         </label>
 
-    <p>
+        <div class="field">
             <button type="submit" class="button is-link">{% trans "Set goal" %}</button>
-        {% if goal %}
+        </div>
-        {% trans "Cancel" as button_text %}
-        {% include 'snippets/toggle/close_button.html' with text=button_text controls_text="show_edit_goal" %}
-        {% endif %}
-    </p>
     </form>
+</div>

bookwyrm/templates/user/goal.html

@@ -1,5 +1,6 @@
 {% extends 'user/layout.html' %}
 {% load i18n %}
+{% load utilities %}
 
 {% block header %}
 <div class="columns is-mobile">
@@ -19,20 +20,8 @@
 <section class="block">
     {% now 'Y' as current_year %}
     {% if user == request.user and year|add:0 == current_year|add:0 %}
-    <div class="block">
+    {% comparison_bool goal None as visible %}
-        <section class="card {% if goal %}is-hidden{% endif %}" id="show_edit_goal">
+    {% include 'user/goal_form.html' with goal=goal year=year visible=visible controls_text="show_edit_goal" class="block" %}
-            <header class="card-header">
-                <h2 class="card-header-title has-background-primary has-text-white" tabindex="0" id="edit_form_header">
-                    <span class="icon icon-book is-size-3 mr-2" aria-hidden="true"></span> {% blocktrans %}{{ year }} Reading Goal{% endblocktrans %}
-                </h2>
-            </header>
-            <section class="card-content content">
-                <p>{% blocktrans %}Set a goal for how many books you'll finish reading in {{ year }}, and track your progress throughout the year.{% endblocktrans %}</p>
-
-                {% include 'snippets/goal_form.html' with goal=goal year=year %}
-            </section>
-        </section>
-    </div>
     {% endif %}
 
     {% if not goal and user != request.user %}

bookwyrm/templates/user/goal_form.html

@@ -0,0 +1,12 @@
+{% extends 'components/inline_form.html' %}
+{% load i18n %}
+
+{% block header %}
+<span class="icon icon-book is-size-3 mr-2" aria-hidden="true"></span>
+{% blocktrans %}{{ year }} Reading Goal{% endblocktrans %}
+{% endblock %}
+
+{% block form %}
+{% include "snippets/goal_form.html" %}
+{% endblock %}
+

bookwyrm/tests/views/test_goal.py

@@ -1,12 +1,13 @@
 """ test for app action functionality """
 from unittest.mock import patch
-from django.utils import timezone
+from tidylib import tidy_document
 
 from django.contrib.auth.models import AnonymousUser
 from django.http import Http404
 from django.template.response import TemplateResponse
 from django.test import TestCase
 from django.test.client import RequestFactory
+from django.utils import timezone
 
 from bookwyrm import models, views
 
@@ -61,7 +62,16 @@ class GoalViews(TestCase):
         request.user = self.local_user
 
         result = view(request, self.local_user.localname, self.year)
-        result.render()
+        html = result.render()
+        _, errors = tidy_document(
+            html.content,
+            options={
+                "drop-empty-elements": False,
+                "warn-proprietary-attributes": False,
+            },
+        )
+        if errors:
+            raise Exception(errors)
         self.assertIsInstance(result, TemplateResponse)
 
     def test_goal_page_anonymous(self):
@@ -92,7 +102,16 @@ class GoalViews(TestCase):
         request.user = self.rat
 
         result = view(request, self.local_user.localname, timezone.now().year)
-        result.render()
+        html = result.render()
+        _, errors = tidy_document(
+            html.content,
+            options={
+                "drop-empty-elements": False,
+                "warn-proprietary-attributes": False,
+            },
+        )
+        if errors:
+            raise Exception(errors)
         self.assertIsInstance(result, TemplateResponse)
 
     def test_goal_page_private(self):