From c2cb187d91d84c3090484844928528a9f7ad4863 Mon Sep 17 00:00:00 2001
From: Joseph Nuthalapati <njoseph@thoughtworks.com>
Date: Fri, 26 Jul 2019 15:31:55 +0530
Subject: [PATCH] backups: Make UI more consistent with other apps

- Replace subsubmenu with buttons
- Left align icons in table header (restore and delete)
- Replace the technical term "repository" with "location"
- Confirmation dialogs shouldn't have app name, description, manual etc.
- Confirmation dialogs don't need Cancel buttons

Signed-off-by: Joseph Nuthalapati <njoseph@thoughtworks.com>
[sunil@medhas.org: Remove 'primary' for all but one button]
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Sunil Mohan Adapa <sunil@medhas.org>
---
 plinth/modules/backups/templates/backups.html | 24 +++++++++++++++----
 .../backups/templates/backups_repository.inc  | 13 ++++++----
 .../templates/backups_repository_add.html     |  5 +---
 .../templates/backups_repository_remove.html  |  6 +----
 .../backups/templates/backups_restore.html    |  4 ----
 .../backups/templates/verify_ssh_hostkey.html |  3 ---
 plinth/modules/backups/views.py               | 24 ++-----------------
 7 files changed, 31 insertions(+), 48 deletions(-)

diff --git a/plinth/modules/backups/templates/backups.html b/plinth/modules/backups/templates/backups.html
index ec33b46a3..e67fb0042 100644
--- a/plinth/modules/backups/templates/backups.html
+++ b/plinth/modules/backups/templates/backups.html
@@ -44,7 +44,21 @@
 
 {% block configuration %}
 
-  <h3>{% trans 'Existing backup locations' %}</h3>
+  <a title="{% trans 'Create a new backup' %}"
+     role="button" class="btn btn-primary"
+     href="{% url 'backups:create' %}">
+    <span class="fa fa-plus" aria-hidden="true"></span>
+    {% trans 'Create Backup' %}
+  </a>
+
+  <a title="{% trans 'Upload and restore a backup archive' %}"
+     role="button" class="btn btn-default"
+     href="{% url 'backups:upload' %}">
+    <span class="fa fa-upload" aria-hidden="true"></span>
+    {% trans 'Upload and Restore' %}
+  </a>
+
+  <h3>{% trans 'Existing Backups' %}</h3>
 
   {% include "backups_repository.inc" with repository=root_repository uuid='root' editable=False %}
 
@@ -52,13 +66,13 @@
     {% include "backups_repository.inc" with editable=True %}
   {% endfor %}
 
-  <br />
+  <br/>
 
-  <a title="{% trans 'Create new repository' %}"
-     role="button" class="btn btn-primary"
+  <a title="{% trans 'Add a remote backup location' %}"
+     role="button" class="btn btn-default"
      href="{% url 'backups:repository-add' %}">
     <span class="fa fa-plus" aria-hidden="true"></span>
-    {% trans 'Add Remote Repository' %}
+    {% trans 'Add Remote Location' %}
   </a>
 
 {% endblock %}
diff --git a/plinth/modules/backups/templates/backups_repository.inc b/plinth/modules/backups/templates/backups_repository.inc
index e73b8e713..6b4752884 100644
--- a/plinth/modules/backups/templates/backups_repository.inc
+++ b/plinth/modules/backups/templates/backups_repository.inc
@@ -24,15 +24,16 @@
   <thead>
     <tr>
       <th colspan="2">
-
+        <span style="vertical-align: text-top">
         {% if repository.error %}
-           <span class="fa fa-exclamation-triangle mount-error"
-                 aria-hidden="true" title="{{ repository.error }}">
-           </span>
+          <span class="fa fa-exclamation-triangle mount-error"
+                aria-hidden="true" title="{{ repository.error }}">
+          </span>
         {% endif %}
 
         {{ repository.name }}
 
+        <span class="pull-right">
         {% if editable %}
 
           {% if repository.mounted %}
@@ -65,11 +66,13 @@
           <a title="{% trans 'Remove Location. This will not delete the remote backup.' %}"
              role="button" class="repository-remove btn btn-sm btn-default"
              href="{% url 'backups:repository-remove' uuid %}">
-             <span class="fa fa-trash-o" aria-hidden="true">
+            <span class="fa fa-trash-o" aria-hidden="true">
           </a>
 
         {% endif %}
+        </span>
 
+        </span>
       </th>
     </tr>
   </thead>
diff --git a/plinth/modules/backups/templates/backups_repository_add.html b/plinth/modules/backups/templates/backups_repository_add.html
index 5e9ae45ae..887f89be6 100644
--- a/plinth/modules/backups/templates/backups_repository_add.html
+++ b/plinth/modules/backups/templates/backups_repository_add.html
@@ -40,10 +40,7 @@
     </div>
 
     <input type="submit" class="btn btn-primary"
-           value="{% trans "Create Repository" %}"/>
-    <a class="btn btn-default" role="button" href="{% url 'backups:index' %}">
-      {% trans "Cancel" %}
-    </a>
+           value="{% trans "Create Location" %}"/>
   </form>
 
 {% endblock %}
diff --git a/plinth/modules/backups/templates/backups_repository_remove.html b/plinth/modules/backups/templates/backups_repository_remove.html
index 3509c80c7..165c70184 100644
--- a/plinth/modules/backups/templates/backups_repository_remove.html
+++ b/plinth/modules/backups/templates/backups_repository_remove.html
@@ -43,11 +43,7 @@
       {% csrf_token %}
 
       <input type="submit" class="btn btn-danger"
-             value="{% trans "Remove Repository" %}"/>
-      <a class="btn btn-default" role="button"
-         href="{% url 'backups:index' %}">
-        {% trans "Cancel" %}
-      </a>
+             value="{% trans "Remove Location" %}"/>
     </form>
   </p>
 
diff --git a/plinth/modules/backups/templates/backups_restore.html b/plinth/modules/backups/templates/backups_restore.html
index 6b58ccf16..dde2ffe8f 100644
--- a/plinth/modules/backups/templates/backups_restore.html
+++ b/plinth/modules/backups/templates/backups_restore.html
@@ -47,10 +47,6 @@
         {% trans "Restoring" %}
       </button>
 
-      <a class="btn btn-default" role="button"
-         href="{% url 'backups:index' %}">
-        {% trans "Cancel" %}
-      </a>
     </form>
   </p>
 
diff --git a/plinth/modules/backups/templates/verify_ssh_hostkey.html b/plinth/modules/backups/templates/verify_ssh_hostkey.html
index c0feb5d68..91ec0e558 100644
--- a/plinth/modules/backups/templates/verify_ssh_hostkey.html
+++ b/plinth/modules/backups/templates/verify_ssh_hostkey.html
@@ -57,9 +57,6 @@
     {{ form|bootstrap }}
 
     <input type="submit" class="btn btn-primary" value="{% trans 'Verify Host' %}"/>
-    <a class="btn btn-default" role="button" href="{% url 'backups:index' %}">
-      {% trans "Cancel" %}
-    </a>
   </form>
 
 {% endblock %}
diff --git a/plinth/modules/backups/views.py b/plinth/modules/backups/views.py
index 200238335..7c62c2b3f 100644
--- a/plinth/modules/backups/views.py
+++ b/plinth/modules/backups/views.py
@@ -49,17 +49,6 @@ from .repository import (BorgRepository, SshBorgRepository, get_repository,
 
 logger = logging.getLogger(__name__)
 
-subsubmenu = [{
-    'url': reverse_lazy('backups:index'),
-    'text': ugettext_lazy('Backups')
-}, {
-    'url': reverse_lazy('backups:upload'),
-    'text': ugettext_lazy('Upload')
-}, {
-    'url': reverse_lazy('backups:create'),
-    'text': ugettext_lazy('Create')
-}]
-
 
 @method_decorator(delete_tmp_backup_file, name='dispatch')
 class IndexView(TemplateView):
@@ -71,7 +60,6 @@ class IndexView(TemplateView):
         context = super().get_context_data(**kwargs)
         context['title'] = backups.name
         context['description'] = backups.description
-        context['subsubmenu'] = subsubmenu
         context['manual_page'] = backups.manual_page
         root_repository = BorgRepository(ROOT_REPOSITORY)
         context['root_repository'] = root_repository.get_view_content()
@@ -90,10 +78,7 @@ class CreateArchiveView(SuccessMessageMixin, FormView):
     def get_context_data(self, **kwargs):
         """Return additional context for rendering the template."""
         context = super().get_context_data(**kwargs)
-        context['title'] = backups.name
-        context['description'] = backups.description
-        context['subsubmenu'] = subsubmenu
-        context['manual_page'] = backups.manual_page
+        context['title'] = _('Create a new backup')
         return context
 
     def form_valid(self, form):
@@ -140,10 +125,7 @@ class UploadArchiveView(SuccessMessageMixin, FormView):
     def get_context_data(self, **kwargs):
         """Return additional context for rendering the template."""
         context = super().get_context_data(**kwargs)
-        context['title'] = backups.name
-        context['description'] = backups.description
-        context['subsubmenu'] = subsubmenu
-        context['manual_page'] = backups.manual_page
+        context['title'] = _('Upload and restore a backup')
         try:
             disk_info = storage.get_disk_info('/')
         except PlinthError as exception:
@@ -272,7 +254,6 @@ class AddRepositoryView(SuccessMessageMixin, FormView):
         """Return additional context for rendering the template."""
         context = super().get_context_data(**kwargs)
         context['title'] = _('Create remote backup repository')
-        context['subsubmenu'] = subsubmenu
         return context
 
     def form_valid(self, form):
@@ -315,7 +296,6 @@ class VerifySshHostkeyView(SuccessMessageMixin, FormView):
         """Return additional context for rendering the template."""
         context = super().get_context_data(**kwargs)
         context['title'] = _('Verify SSH hostkey')
-        context['subsubmenu'] = subsubmenu
         context['hostname'] = self._get_hostname()
         return context