From 33004f62bd45457a43d121ad4ab219155ca9f5f3 Mon Sep 17 00:00:00 2001
From: cryptogopher <cryptogopher@noreply.me>
Date: Sun, 22 Mar 2026 01:06:38 +0100
Subject: [PATCH] Improve Measurement form layout and styling

---
 app/assets/stylesheets/application.css | 187 +++++++++++++------------
 app/helpers/application_helper.rb      |   3 +-
 app/helpers/quantities_helper.rb       |   4 +-
 app/views/default/units/index.html.erb |   2 +-
 app/views/measurements/_form.html.erb  |  49 ++++---
 app/views/quantities/index.html.erb    |   3 +-
 app/views/readouts/_form.html.erb      |  15 +-
 app/views/units/index.html.erb         |   3 +-
 app/views/users/index.html.erb         |   2 +-
 config/locales/en.yml                  |   6 +-
 10 files changed, 150 insertions(+), 124 deletions(-)

diff --git a/app/assets/stylesheets/application.css b/app/assets/stylesheets/application.css
index 97ceb44..f6404da 100644
--- a/app/assets/stylesheets/application.css
+++ b/app/assets/stylesheets/application.css
@@ -58,16 +58,18 @@
   outline: none;
 }
 /* NOTE: move to higher priority layer instead of using !important? */
+/* TODO: add CSS @layer requirements in README */
 [disabled] {
   border-color: var(--color-border-gray) !important;
   color: var(--color-border-gray) !important;
-  /* NOTE: cannot set cursor with `pointer-events: none`; can be fixed by setting
+  /* NOTE: cannot set cursor when `pointer-events: none`; can be fixed by setting
    * `cursor` on wrapping element.
   cursor: not-allowed; */
   fill: var(--color-border-gray) !important;
   pointer-events: none !important;
 }
-/* [hidden] submit elements cannot have `display` set as it makes them visible. */
+/* Styles set `display` without distinguishing between [hidden] elements, making
+ * them visible. */
 [hidden] {
   display: none !important;
 }
@@ -122,6 +124,7 @@ input[type=checkbox]:checked {
 input[type=number] {
   appearance: textfield;
   -moz-appearance: textfield;
+  text-align: end;
 }
 input::-webkit-inner-spin-button {
   -webkit-appearance: none;
@@ -130,6 +133,26 @@ input::-webkit-outer-spin-button {
   -webkit-appearance: none;
   margin: 0;
 }
+/* Text color of table form controls:
+ *  - black for row/table forms,
+ *  - inherited for internal (column specific) buttons/forms. */
+table input,
+table select,
+table summary,
+table textarea {
+  border-color: var(--color-border-gray);
+}
+table input,
+table select,
+table textarea {
+  padding-block: 0.375em;
+}
+table form input,
+table form select,
+table form summary,
+table form textarea {
+  color: inherit;
+}
 input:focus-visible,
 select:focus-visible,
 select:focus-within,
@@ -138,6 +161,7 @@ summary:focus-visible,
 textarea:focus-visible {
   accent-color: var(--color-dark-blue);
   background-color: var(--color-focus-gray);
+  color: black;
 }
 input:hover,
 select:hover,
@@ -148,8 +172,15 @@ textarea:hover {
 }
 select:hover,
 summary:hover {
+  color: black;
   cursor: pointer;
 }
+/* TODO: style <details>/<summary> focus to match <select> as much as possible.
+summary:focus-visible::before,
+summary:hover::before {
+  background-color: black;
+}
+ */
 input:invalid,
 select:invalid,
 textarea:invalid {
@@ -213,9 +244,17 @@ textarea:invalid {
   background-color: var(--color-red);
   border-color: var(--color-red);
 }
+table .button {
+  border-color: var(--color-border-gray);
+  color: var(--color-table-gray);
+  font-weight: normal;
+  height: 100%;
+  padding: 0.4em;
+}
 /* TODO: move normal, non-button links (<a>:hover/:focus) styling here (i.e.
  * page-wide, top-level) and remove from `table.items` - as the style should be
  * same everywhere. */
+/* TODO: apply link class to non-button/-tab links. Add light underscore for links? */
 input[type=text]:read-only,
 textarea:read-only {
   border: none;
@@ -390,79 +429,87 @@ header {
   padding-block: 0;
 }
 
+.tabular-form table {
+  border: none;
+}
+.tabular-form table td {
+  border: none;
+  text-align: left;
+  vertical-align: middle;
+}
+.tabular-form table td:first-child {
+  color: inherit;
+}
 
-/* TODO: remove `.items` class (?) and make `form table` work properly. */
-table.items {
+
+.items-table {
   border-spacing: 0;
   border: 1px solid var(--color-border-gray);
   border-radius: 0.25em;
   font-size: 0.85rem;
   text-align: left;
 }
-table:not(:has(tr)) {
-  display: none;
-}
-table.items thead {
+.items-table thead {
   font-size: 0.8rem;
 }
-table.items thead,
-table.items tbody tr:hover {
+.items-table thead,
+.items-table tbody tr:hover {
   background-color: var(--color-focus-gray);
 }
-table.items th {
+.items-table th {
   padding-block: 0.75em;
   text-align: center;
 }
-table.items th,
-table.items td {
+.items-table th,
+.items-table td {
   padding-inline: 1em 0;
 }
 /* For <a> to fill <td> completely, we use an `::after` pseudoelement. */
-table.items td.link {
+.items-table td.link {
   padding: 0;
   position: relative;
 }
-table.items td.link a {
+.items-table td.link a {
   color: inherit;
   font: inherit;
 }
-table.items td.link a::after {
+.items-table td.link a::after {
   content: '';
   inset: 0;
   position: absolute;
 }
-table.items td:first-child {
+.items-table td:first-child {
   padding-inline-start: calc(1em + var(--depth) * 0.8em);
 }
-table.items td:has(input, select, textarea) {
+.items-table td:has(input, select, textarea) {
   padding-inline-start: calc(0.6em - 0.9px);
 }
-table.items td:first-child:has(input, select, textarea) {
+.items-table td:first-child:has(input, select, textarea) {
   padding-inline-start: calc(0.6em + var(--depth) * 0.8em - 0.9px);
 }
-table.items th:last-child {
+.items-table th:last-child {
   padding-inline-end: 0.4em;
 }
-table.items td:last-child {
+.items-table td:last-child {
   padding-inline-end: 0.1em;
 }
-table.items td {
+.items-table td {
   border-top: 1px solid var(--color-border-gray);
   height: 2.4em;
   padding-block: 0.1em;
 }
-table.items .actions {
+.items-table .actions {
   display: flex;
   gap: 0.4em;
   justify-content: end;
 }
-table.items .actions.centered {
+.items-table .actions.centered {
   justify-content: center;
 }
-table.items tr.dropzone {
+.items-table tr.dropzone {
   position: relative;
 }
-table.items tr.dropzone::after {
+.items-table tr.dropzone::after {
   content: '';
   inset: 1px 0 0 0;
   position: absolute;
@@ -470,96 +517,52 @@ table.items tr.dropzone::after {
   outline-offset: -1px;
   z-index: var(--z-index-table-row-outline);
 }
-table.items td.handle {
+.items-table td.handle {
   cursor: move;
 }
-table.items tr.form td {
+.items-table tr.form td {
   vertical-align: top;
 }
 
 /* TODO: replace `:hover:focus-visible` combos with proper LOVE style order. */
 /* TODO: update table styling: simplify selectors, deduplicate, remove non-font rem. */
-table.items td.link a:hover,
-table.items td.link a:focus-visible,
-table.items td.link a:hover:focus-visible {
+.items-table td.link a:hover,
+.items-table td.link a:focus-visible,
+.items-table td.link a:hover:focus-visible {
   text-decoration: underline;
   text-decoration-thickness: 0.05rem;
   text-underline-offset: 0.2rem;
 }
-table.items td.link a:hover {
+.items-table td.link a:hover {
   color: var(--color-blue);
 }
-table.items td.link a:focus-visible {
+.items-table td.link a:focus-visible {
   text-decoration-color: var(--color-gray);
 }
-table.items td.link a:hover:focus-visible {
+.items-table td.link a:hover:focus-visible {
   color: var(--color-dark-blue);
 }
 
-table.items td:not(:first-child),
+.items-table td:not(:first-child),
 .grayed {
   color: var(--color-table-gray);
   fill: var(--color-table-gray);
 }
-table.items svg {
+.items-table svg {
   height: 1rem;
   vertical-align: middle;
   width: 1rem;
 }
-table.items svg:last-child {
+.items-table svg:last-child {
   height: 1.2rem;
   width: 1.2rem;
 }
-table.items td.svg {
+.items-table td.svg {
   text-align: center;
 }
-table.items td.number {
+.items-table td.number {
   text-align: right;
 }
-table.items .button {
-  font-weight: normal;
-  height: 100%;
-  padding: 0.4em;
-}
-table.items input:not([type=submit]):not([type=checkbox]),
-table.items select,
-table.items textarea {
-  padding-block: 0.375em;
-}
-
-table input,
-table select,
-table summary,
-table textarea {
-  border-color: var(--color-border-gray);
-}
-table select {
-  color: var(--color-table-gray);
-}
-table select:hover,
-table select:focus-within,
-table select:focus-visible {
-  color: black;
-}
-table .button {
-  border-color: var(--color-border-gray);
-  color: var(--color-table-gray);
-}
-
-form table.items {
-  border: none;
-}
-form table.items td {
-  border: none;
-  text-align: left;
-  vertical-align: middle;
-}
-form table.items td:first-child {
-  color: inherit;
-}
-form table select {
-  color: black;
-}
 
 
 .centered {
@@ -587,6 +590,13 @@ form table select {
   font-size: 0.9rem;
   text-align: center;
 }
+.hmin50 {
+  min-width: 50%;
+}
+.italic {
+  color: var(--color-gray);
+  font-style: italic;
+}
 .vflex {
   display: flex;
   gap: 0.8em;
@@ -602,12 +612,12 @@ summary {
   align-items: center;
   color: var(--color-gray);
   display: flex;
-  gap: 0.2em;
+  gap: 0.4em;
   height: 100%;
   white-space: nowrap;
 }
 summary::before {
-  background-color: #000;
+  background-color: currentColor;
   content: "";
   height: 1em;
   mask-image: url('pictograms/chevron-down.svg');
@@ -630,7 +640,7 @@ summary span {
   width: 100%;
 }
 details[open] summary::before {
-  transform: rotate(180deg);
+  transform: scaleY(-1);
 }
 summary::marker {
   padding-left: 0.25em;
@@ -660,6 +670,11 @@ li input[type=checkbox] {
 li::marker {
   content: '';
 }
+
+#measurement_form {
+  min-width: 66%;
+  width: max-content;
+}
 /*
  * TODO:
  * * disable <label> containing disabled checkbox: `label:has(input[disabled])`,
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index e8c3eaf..c78608e 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -152,7 +152,8 @@ module ApplicationHelper
   end
 
   def tabular_form_with(**options, &block)
-    extra_options = {builder: TabularFormBuilder, html: {autocomplete: 'off'}}
+    extra_options = {builder: TabularFormBuilder, class: 'tabular-form',
+                     html: {autocomplete: 'off'}}
     form_with(**merge_attributes(options, extra_options), &block)
   end
 
diff --git a/app/helpers/quantities_helper.rb b/app/helpers/quantities_helper.rb
index 2bb64b9..c0f2547 100644
--- a/app/helpers/quantities_helper.rb
+++ b/app/helpers/quantities_helper.rb
@@ -1,9 +1,9 @@
 module QuantitiesHelper
-  def quantities_check_boxes
+  def quantities_check_boxes(quantities)
     # Closing <details> on focusout event depends on relatedTarget for internal
     # actions being non-null. To ensure this, all top-layer elements of
     # ::details-content must accept focus, e.g. <label> needs tabindex="-1" */
-    collection_check_boxes(nil, :quantity, @quantities, :id, :to_s_with_depth,
+    collection_check_boxes(nil, :quantity, quantities, :id, :to_s_with_depth,
                            include_hidden: false) do |b|
       content_tag :li, b.label(tabindex: -1) { b.check_box + b.text }
     end
diff --git a/app/views/default/units/index.html.erb b/app/views/default/units/index.html.erb
index f545d3c..fc83824 100644
--- a/app/views/default/units/index.html.erb
+++ b/app/views/default/units/index.html.erb
@@ -8,7 +8,7 @@
     class: 'tools-area' %>
 </div>
 
-<table class="main-area items">
+<table class="main-area items-table">
   <thead>
     <tr>
       <th><%= Unit.human_attribute_name(:symbol) %></th>
diff --git a/app/views/measurements/_form.html.erb b/app/views/measurements/_form.html.erb
index 8f1470a..1794cfe 100644
--- a/app/views/measurements/_form.html.erb
+++ b/app/views/measurements/_form.html.erb
@@ -1,28 +1,37 @@
 <%= tabular_form_with model: Measurement.new, id: :measurement_form,
-  class: 'topside-area vflex', html: {onkeydown: 'formProcessKey(event)'} do |form| %>
-  <table class="items centered">
-    <tbody id="readouts"></tbody>
+  class: 'topside-area vflex centered',
+  html: {onkeydown: 'formProcessKey(event)'} do |form| %>
+
+  <table class="items-table centered">
+    <tbody id="readouts">
+      <%= tabular_fields_for @measurement do |form| %>
+        <tr class="italic">
+          <td class="hexpand hmin50"><%= t '.taken_at_html' %></td>
+          <td colspan="3" class="number">
+            <%= form.datetime_field :taken_at, required: true %>
+          </td>
+        </tr>
+      <% end %>
+    </tbody>
   </table>
 
-  <div class="hflex">
-    <%# TODO: right-click selection %>
-    <details id="quantity_select" class="hexpand" open
-             onkeydown="detailsProcessKey(event)">
-      <summary autofocus>
-        <!-- TODO: Set content with CSS when span empty to avoid duplication -->
-        <span data-prompt="<%= t('.select_quantity') %>">
-          <%= t('.select_quantity') %>
-        </span>
-        <%= image_button_tag t(:apply), "update", name: nil, disabled: true,
-          formaction: new_readout_path, formmethod: :get, formnovalidate: true,
-          data: {turbo_stream: true} %>
-      </summary>
-      <ul><%= quantities_check_boxes %></ul>
-    </details>
-    <%= form.button id: :create_measurement_button, disabled: true -%>
-  </div>
+  <%# TODO: right-click selection; unnecessary with hierarchical tags? %>
+  <details id="quantity_select" class="centered hexpand" open
+           onkeydown="detailsProcessKey(event)">
+    <summary autofocus>
+      <!-- TODO: Set content with CSS when span empty to avoid duplication -->
+      <span data-prompt="<%= t('.select_quantity') %>">
+        <%= t('.select_quantity') %>
+      </span>
+      <%= image_button_tag t(:apply), "update", name: nil, disabled: true,
+        formaction: new_readout_path, formmethod: :get, formnovalidate: true,
+        data: {turbo_stream: true} %>
+    </summary>
+    <ul><%= quantities_check_boxes(@quantities) %></ul>
+  </details>
 
   <div class="hflex reverse">
+    <%= form.button id: :create_measurement_button, disabled: true -%>
     <%= image_link_to t(:cancel), "close-outline", measurements_path, name: :cancel,
       class: 'dangerous', onclick: render_turbo_stream('form_close') %>
   </div>
diff --git a/app/views/quantities/index.html.erb b/app/views/quantities/index.html.erb
index 6d89ed4..9d9a1af 100644
--- a/app/views/quantities/index.html.erb
+++ b/app/views/quantities/index.html.erb
@@ -8,9 +8,10 @@
     class: 'tools-area' %>
 </div>
 
+<%# TODO: remove? form can be inserted directly, e.g. at the end of index %>
 <%= tag.div class: 'main-area', id: :quantity_form %>
 
-<table class="main-area items">
+<table class="main-area items-table">
   <thead>
     <tr>
       <th><%= Quantity.human_attribute_name(:name) %></th>
diff --git a/app/views/readouts/_form.html.erb b/app/views/readouts/_form.html.erb
index 4b930cc..bbb5c31 100644
--- a/app/views/readouts/_form.html.erb
+++ b/app/views/readouts/_form.html.erb
@@ -1,13 +1,8 @@
 <%# TODO: add readout reordering by dragging %>
 <%= tabular_fields_for 'readouts[]', readout do |form| %>
   <%- tag.tr id: dom_id(readout.quantity, :new, :readout) do %>
-    <td class="actions">
-      <%# TODO: change to _link_ after giving up displaying relative paths %>
-      <%= image_button_tag '', 'delete-outline', class: 'dangerous', name: nil,
-        formaction: discard_readouts_path(readout.quantity),
-        formmethod: :get, formnovalidate: true, data: {turbo_stream: true} %>
-    </td>
     <td>
+      <%# TODO: add grayed readout index (in separate column?) %>
       <%= readout.quantity.relative_pathname(@superquantity) %>
     </td>
     <td>
@@ -17,7 +12,13 @@
       <%= form.hidden_field :quantity_id %>
       <%= form.collection_select :unit_id, @user_units, :id,
         ->(u){ sanitize('&emsp;' * (u.base_id ? 1 : 0) + u.symbol) },
-        {prompt: t('.select_unit'), disabled: '', selected: ''}, required: true %>
+        {prompt: '', disabled: '', selected: ''}, required: true %>
+    </td>
+    <td class="actions">
+      <%# TODO: change to _link_ after giving up displaying relative paths %>
+      <%= image_button_tag '', 'delete-outline', class: 'dangerous', name: nil,
+        formaction: discard_readouts_path(readout.quantity),
+        formmethod: :get, formnovalidate: true, data: {turbo_stream: true} %>
     </td>
   <% end %>
 <% end %>
diff --git a/app/views/units/index.html.erb b/app/views/units/index.html.erb
index ffc103d..417fab5 100644
--- a/app/views/units/index.html.erb
+++ b/app/views/units/index.html.erb
@@ -7,9 +7,10 @@
     class: 'tools-area' %>
 </div>
 
+<%# TODO: remove? form can be inserted directly, e.g. at the end of index %>
 <%= tag.div id: :unit_form %>
 
-<table class="main-area items">
+<table class="main-area items-table">
   <thead>
     <tr>
       <th><%= Unit.human_attribute_name(:symbol) %></th>
diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb
index 039abe8..ccb438d 100644
--- a/app/views/users/index.html.erb
+++ b/app/views/users/index.html.erb
@@ -1,4 +1,4 @@
-<table class="main-area items" id="users">
+<table class="main-area items-table" id="users">
   <thead>
     <tr>
       <th><%= User.human_attribute_name(:email) %></th>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 24d5395..3382167 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -85,12 +85,10 @@ en:
     navigation: Measurements
     no_items: There are no measurements taken. You can Add some now.
     form:
-      select_quantity: select the measured quantities...
+      select_quantity: select quantities...
+      taken_at_html: Measurement taken at&emsp;
     index:
       new_measurement: Add measurement
-  readouts:
-    form:
-      select_unit: ...
   quantities:
     navigation: Quantities
     no_items: There are no configured quantities. You can Add some or Import from defaults.