From cf826b38fe166c1ed0beeafe3485ce8a4ac95dc6 Mon Sep 17 00:00:00 2001
From: Joe Roe <joe@joeroe.io>
Date: Mon, 20 Nov 2023 13:11:41 +0100
Subject: [PATCH] Switch to inline editing for sites

---
 app/controllers/sites_controller.rb |  2 +-
 app/helpers/application_helper.rb   |  8 ++++++
 app/views/sites/_fields.html.erb    | 12 ++++-----
 app/views/sites/_form.html.erb      |  7 -----
 app/views/sites/_header.html.erb    | 10 +++++++
 app/views/sites/edit.html.erb       | 28 ++++++++++++++++---
 app/views/sites/show.html.erb       | 42 ++++++++++++-----------------
 7 files changed, 66 insertions(+), 43 deletions(-)
 create mode 100644 app/views/sites/_header.html.erb

diff --git a/app/controllers/sites_controller.rb b/app/controllers/sites_controller.rb
index 7b9c2ecd..35cf3fcb 100644
--- a/app/controllers/sites_controller.rb
+++ b/app/controllers/sites_controller.rb
@@ -107,7 +107,7 @@ def create
   def update
     respond_to do |format|
       if @site.update(site_params)
-        format.html { redirect_back(fallback_location: @site, notice: "Saved changes to #{@site.name}.") }
+        format.html { redirect_to @site, notice: "Saved changes to #{@site.name}." }
         format.json { render :show, status: :ok, location: @site }
       else
         format.html { render :edit }
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 733d2448..22ecc1fb 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -37,6 +37,14 @@ def delete_icon(options = {})
     bs_icon "trash-fill", options
   end
 
+  def confirm_icon(options = {})
+    bs_icon "check", options
+  end
+
+  def cancel_icon(options = {})
+    bs_icon "x", options
+  end
+
   def active_class(path)
     "active" if current_page?(path)
   end
diff --git a/app/views/sites/_fields.html.erb b/app/views/sites/_fields.html.erb
index 4743a25a..ae0bd81e 100644
--- a/app/views/sites/_fields.html.erb
+++ b/app/views/sites/_fields.html.erb
@@ -1,11 +1,5 @@
 <%= f.text_field :name %>
 
-<div class="row">
-  <div class="col"><%= f.number_field :lat, label: 'Latitude', append: '°N',
-    help: '<a href="https://www.fcc.gov/media/radio/dms-decimal" target="_blank">Convert DMS to decimal degrees</a>.'.html_safe %></div>
-  <div class="col"><%= f.number_field :lng, label: 'Longitude', append: '°E' %></div>
-</div>
-
 <%# country_select is not supported by bootstrap_forms gem, so we need to 
     construct the label and wrapper HTML manually here %>
 <div class="mb-3">
@@ -25,3 +19,9 @@
   { label: "Site type(s)", include_blank: na_value },
   { multiple: true, data: { controller: "tom-select" } }
 %>
+
+<div class="row">
+  <div class="col"><%= f.number_field :lat, label: 'Latitude', append: '°N',
+    help: '<a href="https://www.fcc.gov/media/radio/dms-decimal" target="_blank">Convert DMS to decimal degrees</a>.'.html_safe %></div>
+  <div class="col"><%= f.number_field :lng, label: 'Longitude', append: '°E' %></div>
+</div>
diff --git a/app/views/sites/_form.html.erb b/app/views/sites/_form.html.erb
index 790a2868..453fa49e 100644
--- a/app/views/sites/_form.html.erb
+++ b/app/views/sites/_form.html.erb
@@ -1,11 +1,4 @@
 <%= bootstrap_form_with(model: @site, local: true, id: "siteForm") do |f| %>
-
   <%= render 'fields', f: f %>
-
-  <%= content_for :buttons do %>
-    <%= button_tag "Cancel", type: 'reset', class: 'btn btn-danger', form: "siteForm", data: { "bs-dismiss": "modal" } %>
-    <%= button_tag "Save", class: 'btn btn-success', form: "siteForm" %>
-  <% end %>
-
 <% end %>
 
diff --git a/app/views/sites/_header.html.erb b/app/views/sites/_header.html.erb
new file mode 100644
index 00000000..78ac5444
--- /dev/null
+++ b/app/views/sites/_header.html.erb
@@ -0,0 +1,10 @@
+<header id="site-header" class="bg-light text-primary">
+  <div class="container my-5">
+    <h1 class="display-1"><%= site.name %></h1>
+    <h2 class="display-6">
+      <%= xr_icon Site, {}, { style: "width: auto; height: 2.5rem;" } %>
+      <%= Site.label.humanize %> 
+      <% if site.country.present? %> in <%= site.country.common_name %><%end%>
+    </h2>
+  </div>
+</header>
diff --git a/app/views/sites/edit.html.erb b/app/views/sites/edit.html.erb
index 53a63726..dcb31518 100644
--- a/app/views/sites/edit.html.erb
+++ b/app/views/sites/edit.html.erb
@@ -1,6 +1,26 @@
-<%= render "shared/remote_modal", title: "Edit site details" do %>
+<%= render "header", site: @site %>
 
-  <%= render 'form' %>
-
-<% end %>
+<div class="container my-5">
+  <%= turbo_frame_tag @site do %>
+    <div class="d-flex">
+      <h2>
+        <%= xr_icon Site, {}, style: "width: auto; height: 2rem" %>
+        <%= Site.label.humanize %> details
+      </h2>
+      <% if can? :edit, Site %>
+        <div class="ms-auto">
+          <%= link_to cancel_icon, @site,
+            title: "Cancel editing",
+            class: "btn btn-sm btn-outline-danger",
+            form: "siteForm" %>
+          <%= button_tag confirm_icon,
+            title: "Save changes",
+            class: "btn btn-sm btn-success",
+            form: "siteForm" %>
+        </div>
+      <% end %>
+    </div>
+    <%= render 'form' %>
+  <% end %>
+</div>
 
diff --git a/app/views/sites/show.html.erb b/app/views/sites/show.html.erb
index 9a8ef2ac..b6b73d12 100644
--- a/app/views/sites/show.html.erb
+++ b/app/views/sites/show.html.erb
@@ -1,16 +1,7 @@
 <% content_for :title, @site.name %>
 <% content_for :meta_description, "Chronometric data for #{@site.name}, an archaeological site in #{@site.country}, including #{@site.c14s.count} radiocarbon dates, #{@site.typos.count} typological classifications, and #{@site.references.count} bibliographic references." %>
 
-<header id="site-header" class="bg-light text-primary">
-  <div class="container my-5">
-    <h1 class="display-1"><%= @site.name %></h1>
-    <h2 class="display-6">
-      <%= xr_icon Site, {}, { style: "height: 2.5rem;" } %>
-      <%= Site.label.humanize %> 
-      <% if @site.country.present? %> in <%= @site.country.common_name %><%end%>
-    </h2>
-  </div>
-</header>
+<%= render "header", site: @site %>
 
 <% unless @site.lat.blank? or @site.lng.blank? %>
   <section id="siteMap" class="bg-primary">
@@ -26,21 +17,22 @@
 
 <section id="site-details" class="bg-body">
   <div class="container my-5">
-    <div class="d-flex">
-      <h2>
-        <%= xr_icon Site, {}, style: "width: auto; height: 2rem" %>
-        <%= Site.label.humanize %> details
-      </h2>
-      <% if can? :edit, Site %>
-        <div class="ms-auto">
-          <%= link_to edit_icon,
-            edit_site_path, 
-            class: "btn btn-sm btn-outline-primary",
-            data: { "turbo_frame": "remote_modal" } %>
-        </div>
-      <% end %>
-    </div>
-    <%= render "site_details", site: @site %>
+    <%= turbo_frame_tag @site do %>
+      <div class="d-flex">
+        <h2>
+          <%= xr_icon Site, {}, style: "width: auto; height: 2rem" %>
+          <%= Site.label.humanize %> details
+        </h2>
+        <% if can? :edit, Site %>
+          <div class="ms-auto">
+            <%= link_to edit_icon,
+              edit_site_path, 
+              class: "btn btn-sm btn-outline-primary" %>
+          </div>
+        <% end %>
+      </div>
+      <%= render "site_details", site: @site %>
+    <% end %>
   </div>
 </section>