Tell a person when their column layout was not saved

CHANGELOG.md

@@ -4,6 +4,9 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+### Added
+- A data table's Columns menu says when a change was not saved, puts its boxes and order back to what the table shows, and does not reload the page.
+
 ### Fixed
 - A data table treats a saved layout that is not a hash, or a hidden list or column order that is not a list, as nothing saved, instead of raising an error or reading text as a column name.
 

README.md

@@ -522,7 +522,7 @@ Renders a "Columns" dropdown button with checkboxes for showing/hiding `hideable
 - `hidden_columns:` (Array) — currently hidden column keys
 - `save_url:` (String) — PATCH endpoint to persist preferences; omit for no persistence
 
-Each column in the menu has an up and a down button that move it one place; the first column's up button and the last column's down button are disabled. Ticking, unticking and moving change only the open menu, and an unticked column's name turns grey straight away. When the menu closes, by a click outside it or on its Columns button, the Stimulus `column-picker` controller PATCHes `{ "hidden_columns": [...], "column_order": [...] }` as JSON to `save_url` once, then reloads via `Turbo.visit`. Closing it with nothing changed sends nothing.
+Each column in the menu has an up and a down button that move it one place; the first column's up button and the last column's down button are disabled. Ticking, unticking and moving change only the open menu, and an unticked column's name turns grey straight away. When the menu closes, by a click outside it or on its Columns button, the Stimulus `column-picker` controller PATCHes `{ "hidden_columns": [...], "column_order": [...] }` as JSON to `save_url` once, then reloads via `Turbo.visit`. Closing it with nothing changed sends nothing. When the save is refused or cannot reach the server, the menu says "Your column changes were not saved.", puts its boxes and order back to what the table shows, and does not reload the page.
 
 ```erb
 <%= ui_column_picker(

app/assets/javascripts/keystone_ui/column_picker_controller.js

@@ -1,10 +1,11 @@
 import { Controller } from "@hotwired/stimulus"
 
 export default class extends Controller {
-  static targets = ["menu", "option"]
+  static targets = ["menu", "option", "error"]
   static values = { saveUrl: String }
 
   connect() {
+    this.shown = this.optionTargets.map(option => [ option, this.checkboxIn(option).checked ])
     this._close = this.close.bind(this)
     document.addEventListener("click", this._close)
   }
@@ -66,6 +67,10 @@ export default class extends Controller {
 
   moved() {
     this.changed = true
+    this.refreshMoveButtons()
+  }
+
+  refreshMoveButtons() {
     const options = this.optionTargets
     options.forEach((option, index) => {
       option.querySelector('[data-action="click->column-picker#moveUp"]').disabled = index === 0
@@ -99,8 +104,20 @@ export default class extends Controller {
         "X-CSRF-Token": token
       },
       body: JSON.stringify({ hidden_columns: this.hiddenColumns(), column_order: this.columnOrder() })
-    }).then(() => {
+    }).then((response) => {
+      if (!response.ok) return this.failed()
+
       Turbo.visit(window.location.href, { action: "replace" })
+    }).catch(() => this.failed())
+  }
+
+  failed() {
+    this.errorTarget.classList.remove("hidden")
+    this.shown.forEach(([ option, checked ]) => {
+      option.parentNode.insertBefore(option, null)
+      this.checkboxIn(option).checked = checked
+      option.querySelector("label").classList.toggle("ks-menu-option-hidden", !checked)
     })
+    this.refreshMoveButtons()
   }
 }

app/components/keystone/ui/column_picker_component.html.erb

@@ -29,4 +29,5 @@
       </div>
     <% end %>
   </div>
+  <p data-column-picker-target="error" role="alert" class="<%= ERROR_CLASSES %>">Your column changes were not saved.</p>
 </div>

app/components/keystone/ui/column_picker_component.rb

@@ -11,6 +11,7 @@ class ColumnPickerComponent < ViewComponent::Base
       OPTION_HIDDEN_CLASSES = "ks-menu-option-hidden"
       OPTION_ROW_CLASSES = "flex items-center"
       MOVE_BUTTON_CLASSES = "ks-menu-move text-sm"
+      ERROR_CLASSES = "ks-error hidden"
 
       COLUMNS_ICON = <<~SVG.freeze
         <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20" fill="currentColor" class="w-4 h-4">

test/javascript/column_picker_controller_test.js

@@ -43,13 +43,15 @@ function pickerWith(columns) {
   })
   const inside = {}
   const menu = { classList: classes() }
+  const error = { classList: classes("hidden") }
   const controller = new ColumnPickerController({ scope: { element: { contains: (target) => target === inside } } })
   Object.defineProperty(controller, "optionTargets", { get: () => options.slice() })
   Object.defineProperty(controller, "menuTarget", { value: menu })
+  Object.defineProperty(controller, "errorTarget", { value: error })
   Object.defineProperty(controller, "hasSaveUrlValue", { value: true })
   Object.defineProperty(controller, "saveUrlValue", { value: "/preferences/months" })
   const on = (index) => ({ currentTarget: { closest: () => options[index] } })
-  return { controller, options, menu, on, outside: { target: {} } }
+  return { controller, options, menu, error, on, outside: { target: {} } }
 }
 
 function sentBodies(run) {
@@ -59,7 +61,7 @@ function sentBodies(run) {
   globalThis.window = { location: { href: "/months" } }
   globalThis.fetch = (url, request) => {
     bodies.push(JSON.parse(request.body))
-    return Promise.resolve()
+    return Promise.resolve({ ok: true })
   }
   run()
   return bodies
@@ -205,3 +207,93 @@ test("closing the menu with its Columns button after a tick sends the hidden col
 
   assert.deepEqual(bodies, [ { hidden_columns: [ "pipeline" ], column_order: [ "outreach", "pipeline" ] } ])
 })
+
+async function afterSaving(answer, run) {
+  const visits = []
+  globalThis.document = { querySelector: () => null, addEventListener() {}, removeEventListener() {} }
+  globalThis.Turbo = { visit: (url) => visits.push(url) }
+  globalThis.window = { location: { href: "/months" } }
+  globalThis.fetch = () => answer()
+  run()
+  await new Promise((resolve) => setImmediate(resolve))
+  return visits
+}
+
+test("a save the server answers with an error shows that the change was not saved", async () => {
+  const { controller, options, error, on, outside } = pickerWith([
+    { key: "outreach", shown: true },
+    { key: "pipeline", shown: true }
+  ])
+  options[1].parts.checkbox.checked = false
+
+  await afterSaving(() => Promise.resolve({ ok: false }), () => {
+    controller.connect()
+    controller.mark(on(1))
+    controller.close(outside)
+  })
+
+  assert.equal(error.classList.contains("hidden"), false)
+})
+
+test("a save that cannot reach the server shows that the change was not saved", async () => {
+  const { controller, options, error, on, outside } = pickerWith([
+    { key: "outreach", shown: true },
+    { key: "pipeline", shown: true }
+  ])
+  options[1].parts.checkbox.checked = false
+
+  await afterSaving(() => Promise.reject(new TypeError("Failed to fetch")), () => {
+    controller.connect()
+    controller.mark(on(1))
+    controller.close(outside)
+  })
+
+  assert.equal(error.classList.contains("hidden"), false)
+})
+
+test("a failed save leaves the page without reloading it", async () => {
+  const { controller, options, on, outside } = pickerWith([
+    { key: "outreach", shown: true },
+    { key: "pipeline", shown: true }
+  ])
+  options[1].parts.checkbox.checked = false
+
+  const visits = await afterSaving(() => Promise.resolve({ ok: false }), () => {
+    controller.connect()
+    controller.mark(on(1))
+    controller.close(outside)
+  })
+
+  assert.deepEqual(visits, [])
+})
+
+test("after a failed save the menu's boxes go back to what the table shows", async () => {
+  const { controller, options, on, outside } = pickerWith([
+    { key: "outreach", shown: true },
+    { key: "pipeline", shown: true }
+  ])
+
+  await afterSaving(() => Promise.resolve({ ok: false }), () => {
+    controller.connect()
+    options[1].parts.checkbox.checked = false
+    controller.mark(on(1))
+    controller.close(outside)
+  })
+
+  assert.deepEqual(options.map(({ parts }) => [ parts.checkbox.checked, parts.label.classList.contains("ks-menu-option-hidden") ]), [ [ true, false ], [ true, false ] ])
+})
+
+test("after a failed save the menu's order goes back to what the table shows", async () => {
+  const { controller, options, on, outside } = pickerWith([
+    { key: "outreach", shown: true },
+    { key: "pipeline", shown: true }
+  ])
+
+  await afterSaving(() => Promise.resolve({ ok: false }), () => {
+    controller.connect()
+    controller.moveUp(on(1))
+    controller.close(outside)
+  })
+
+  assert.deepEqual(options.map(({ parts }) => parts.checkbox.value), [ "outreach", "pipeline" ])
+})

test/render/data_table_component_render_test.rb

@@ -295,6 +295,17 @@ def test_given_no_locked_column_keeps_every_column_scrolling_with_the_table
     assert_empty page.css(".ks-table-header-locked, .ks-table-cell-locked, .sticky")
   end
 
+  def test_holds_a_hidden_message_in_its_columns_menu_for_a_change_that_was_not_saved
+    KeystoneUi.configure { |c| c.preference_supplier = ->(_view, _key) { { value: nil, save_url: "/preferences/months" } } }
+
+    columns = month_columns
+    page = render_in_view_context do
+      ui_data_table(items: [ { month: "Jan", pipeline: "$10" } ], columns: columns, key: :months)
+    end
+
+    assert_equal [ "Your column changes were not saved." ], page.css("[data-column-picker-target=error].hidden[role=alert]").map { |message| message.text.strip }
+  end
+
   private
 
   def three_columns

the_local/agents/keystone_ui-develop.md

@@ -240,8 +240,9 @@ outer element. See Conventions before using it.
   and the hideable ones fill the remaining places. With no `"column_order"` the
   columns keep their declared order. Whenever the supplier
   returns a save address, the table renders a "Columns" menu in a row above
-  itself, aligned right, that saves to it as `ui_column_picker` does, including
-  for a person with nothing saved yet. That menu lists the hideable columns in
+  itself, aligned right, that saves to it as `ui_column_picker` does and shows
+  the same message when a save fails, including for a person with nothing saved
+  yet. That menu lists the hideable columns in
   the order the table shows them and leaves ticked exactly the ones the table
   shows. When the
   supplier returns nothing, when no supplier is set, or when no `key:` is
@@ -271,7 +272,11 @@ outer element. See Conventions before using it.
   `X-CSRF-Token` header, then reloads the page. A menu closed with nothing
   changed sends nothing. `column_order` lists every hideable column's key in
   the menu's order when it closes. With no `save_url:` it sends nothing. The app must provide that endpoint and persist
-  both lists. The picker does not reorder the table: beside a table without
+  both lists. The endpoint must answer with a success status when it has saved
+  them. When it answers with an error status, or the request cannot reach the
+  server, the menu does not reload the page: it shows "Your column changes were
+  not saved." under the Columns button, and puts its boxes and its order back
+  to what the table shows. The picker does not reorder the table: beside a table without
   `key:`, the app must pass the table and the picker its columns in the saved
   order itself. A table
   given `key:` renders its own Columns menu when the supplier gives a save

the_local/agents/keystone_ui-info.md

@@ -155,7 +155,10 @@ holds the catalog.
   columns sends nothing while the menu is open. Closing the menu, with its
   Columns button or by clicking anywhere outside it, saves the hidden columns
   and the order together once, then reloads the page. Closing it with nothing
-  changed saves nothing.
+  changed saves nothing. When the save is refused or cannot reach the server,
+  the page does not reload. The Columns menu shows the message "Your
+  column changes were not saved." and puts its boxes and order back to what
+  the table shows.
   A table with no key, or an app with no supplier, renders from its own call
   alone. Setting up the supplier belongs to the install local.
 - **Suggestions.** A form field can carry a list of suggested values. The

the_local/agents/keystone_ui-install.md

@@ -282,6 +282,11 @@ built on ViewComponent; hook it in before building any screen with those helpers
      string keys, `"hidden_columns"` and `"column_order"`, or the saved order
      is not applied. With no `save_url`, the saved layout applies and no menu
      is shown.
+   - The action at `save_url` must answer a stored layout with a 2xx status.
+     Any other status, or a request that cannot reach the server, leaves the
+     page unreloaded. The table then shows "Your column changes were not
+     saved." beside its Columns button, and the menu's boxes and order go back
+     to what the table shows.
    - A companion preferences gem may set this supplier for the app. Ask the
      developer whether the app uses one, or which code stores each user's table
      layouts, before writing the callable.