From b81bdc778b2f383fd0507b355608a9005cff7f67 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Sat, 21 Aug 2021 23:54:07 +0200
Subject: [PATCH] Move budget actions out of the form

Since we aren't very consistent in the styles for submit buttons in the
admin section, it was possible to accidentally press the "Calculate
Winner Investments" link after filling in the form. This wouldn't submit
the form and so the changes wouldn't be saved.

It's also a bit confusing for users. After filling in a form, users
expect a submit button. When finding two buttons, they've got to stop to
think which one is the one they've got to press.

Furthermore, the "Calculate Winner Investments" link would work better
as a button instead of a link, for the reasons mentioned in commit
5311daadf. Since HTML forms can't be nested, that would mean treating
this button in a different way than the rest of the buttons in the
application.

Moving the link before the form solves the issue.

Since now we've got the budget actions before the budget form, we're
grouping these actions together with the "Preview" action. We're not
adding the "Publish budget" button as well to this group because it's
got an associated message.
---
 .../stylesheets/admin/budgets/actions.scss    | 27 ++++++++++++
 .../stylesheets/admin/budgets/drafting.scss   | 44 ++++++-------------
 .../admin/budgets/actions_component.html.erb  | 15 +++++++
 .../admin/budgets/actions_component.rb        |  7 +++
 .../admin/budgets/drafting_component.html.erb | 20 +++------
 .../admin/budgets/drafting_component.rb       |  4 ++
 .../admin/budgets/form_component.html.erb     | 16 -------
 app/views/admin/budgets/edit.html.erb         |  1 +
 8 files changed, 75 insertions(+), 59 deletions(-)
 create mode 100644 app/assets/stylesheets/admin/budgets/actions.scss
 create mode 100644 app/components/admin/budgets/actions_component.html.erb
 create mode 100644 app/components/admin/budgets/actions_component.rb

diff --git a/app/assets/stylesheets/admin/budgets/actions.scss b/app/assets/stylesheets/admin/budgets/actions.scss
new file mode 100644
index 000000000..e733807c5
--- /dev/null
+++ b/app/assets/stylesheets/admin/budgets/actions.scss
@@ -0,0 +1,27 @@
+.admin .budgets-actions {
+  $gap: $line-height;
+  @include flex-with-gap($gap);
+  align-items: baseline;
+  flex-wrap: wrap;
+  margin-top: -$gap;
+
+  > * {
+    margin-bottom: 0;
+    margin-top: $gap;
+  }
+
+  + * {
+    margin-top: $line-height;
+  }
+
+  .preview-link {
+    @include has-fa-icon(eye, regular);
+    @include hollow-button;
+    margin-left: $gap;
+    margin-top: $gap;
+
+    &::before {
+      margin-#{$global-right}: $font-icon-margin;
+    }
+  }
+}
diff --git a/app/assets/stylesheets/admin/budgets/drafting.scss b/app/assets/stylesheets/admin/budgets/drafting.scss
index 28819e763..2ab00daf6 100644
--- a/app/assets/stylesheets/admin/budgets/drafting.scss
+++ b/app/assets/stylesheets/admin/budgets/drafting.scss
@@ -1,41 +1,25 @@
 .admin .drafting {
-  margin-bottom: 2 * $line-height / 3;
-  margin-left: auto;
+  $vertical-gap: nth($callout-margin, 3);
+  @include flex-with-gap($line-height / 2);
+  align-items: flex-start;
+  flex-wrap: wrap;
+  margin-bottom: $line-height * 1.5;
+  margin-top: -$vertical-gap;
 
-  @include breakpoint(large) {
-    align-items: flex-start;
-    display: flex;
-
-    .callout {
-      flex: 1;
-      margin-bottom: 0;
-    }
+  > * {
+    margin-top: $vertical-gap;
   }
 
-  @include breakpoint(small medium only) {
-    text-align: right;
-
-    .callout {
-      text-align: left;
-    }
-  }
-
-  .preview-link {
-    @include has-fa-icon(eye, regular);
-    @include hollow-button;
-
-    &::before {
-      margin-right: $font-icon-margin;
-    }
+  .callout {
+    flex-basis: $global-width / 3;
+    flex-grow: 1;
+    margin-bottom: 0;
   }
 
   .publish-link {
     @include regular-button;
     margin-bottom: 0;
-  }
-
-  .preview-link,
-  .publish-link {
-    margin-left: $line-height / 2;
+    margin-#{$global-left}: $line-height / 2;
+    margin-top: $vertical-gap;
   }
 }
diff --git a/app/components/admin/budgets/actions_component.html.erb b/app/components/admin/budgets/actions_component.html.erb
new file mode 100644
index 000000000..e638838f3
--- /dev/null
+++ b/app/components/admin/budgets/actions_component.html.erb
@@ -0,0 +1,15 @@
+<div class="budgets-actions">
+  <%= render Admin::Budgets::CalculateWinnersButtonComponent.new(budget) %>
+  <% if budget.has_winning_investments? %>
+    <%= link_to t("budgets.show.see_results"),
+                budget_results_path(budget),
+                class: "button hollow" %>
+  <% end %>
+
+  <%= link_to t("admin.budgets.actions.preview"), budget_path(budget), class: "preview-link", target: "_blank" %>
+
+  <%= link_to t("admin.budgets.edit.delete"),
+      admin_budget_path(budget),
+      method: :delete,
+      class: "delete" %>
+</div>
diff --git a/app/components/admin/budgets/actions_component.rb b/app/components/admin/budgets/actions_component.rb
new file mode 100644
index 000000000..3eb3c64e6
--- /dev/null
+++ b/app/components/admin/budgets/actions_component.rb
@@ -0,0 +1,7 @@
+class Admin::Budgets::ActionsComponent < ApplicationComponent
+  attr_reader :budget
+
+  def initialize(budget)
+    @budget = budget
+  end
+end
diff --git a/app/components/admin/budgets/drafting_component.html.erb b/app/components/admin/budgets/drafting_component.html.erb
index 6c84a4972..bde73a546 100644
--- a/app/components/admin/budgets/drafting_component.html.erb
+++ b/app/components/admin/budgets/drafting_component.html.erb
@@ -1,16 +1,10 @@
 <div class="drafting">
-  <% if can? :publish, budget %>
-    <div class="callout warning">
-      <strong><%= t("admin.budgets.edit.drafting") %></strong>
-    </div>
-  <% end %>
+  <div class="callout warning">
+    <strong><%= t("admin.budgets.edit.drafting") %></strong>
+  </div>
 
-  <%= link_to t("admin.budgets.actions.preview"), budget_path(budget), class: "preview-link", target: "_blank" %>
-
-  <% if can? :publish, budget %>
-    <%= link_to t("admin.budgets.edit.publish"),
-                publish_admin_budget_path(budget),
-                method: :patch, class: "publish-link",
-                data: { confirm: t("admin.actions.confirm") } %>
-  <% end %>
+  <%= link_to t("admin.budgets.edit.publish"),
+              publish_admin_budget_path(budget),
+              method: :patch, class: "publish-link",
+              data: { confirm: t("admin.actions.confirm") } %>
 </div>
diff --git a/app/components/admin/budgets/drafting_component.rb b/app/components/admin/budgets/drafting_component.rb
index 18d37b93f..d196b0184 100644
--- a/app/components/admin/budgets/drafting_component.rb
+++ b/app/components/admin/budgets/drafting_component.rb
@@ -5,4 +5,8 @@ class Admin::Budgets::DraftingComponent < ApplicationComponent
   def initialize(budget)
     @budget = budget
   end
+
+  def render?
+    can?(:publish, budget)
+  end
 end
diff --git a/app/components/admin/budgets/form_component.html.erb b/app/components/admin/budgets/form_component.html.erb
index 6fa6ecec1..fd2026da0 100644
--- a/app/components/admin/budgets/form_component.html.erb
+++ b/app/components/admin/budgets/form_component.html.erb
@@ -84,21 +84,5 @@
         <%= f.submit nil, class: "button success" %>
       <% end %>
     </div>
-
-    <div class="float-right">
-      <%= render Admin::Budgets::CalculateWinnersButtonComponent.new(budget) %>
-
-      <% if budget.has_winning_investments? %>
-        <%= link_to t("budgets.show.see_results"),
-                    budget_results_path(budget),
-                    class: "button hollow margin-left" %>
-      <% end %>
-      <% if budget.persisted? %>
-        <%= link_to t("admin.budgets.edit.delete"),
-            admin_budget_path(budget),
-            method: :delete,
-            class: "delete float-right margin-left" %>
-      <% end %>
-    </div>
   </div>
 <% end %>
diff --git a/app/views/admin/budgets/edit.html.erb b/app/views/admin/budgets/edit.html.erb
index ad5d44f76..07247f80d 100644
--- a/app/views/admin/budgets/edit.html.erb
+++ b/app/views/admin/budgets/edit.html.erb
@@ -5,4 +5,5 @@
   <%= render Admin::Budgets::DraftingComponent.new(@budget) %>
 </header>
 
+<%= render Admin::Budgets::ActionsComponent.new(@budget) %>
 <%= render Admin::Budgets::FormComponent.new(@budget) %>