From b7c14df9c007b51a50990400a30ba765c486ad68 Mon Sep 17 00:00:00 2001 From: tylercschneider Date: Wed, 7 Oct 2026 10:27:19 -0400 Subject: [PATCH 1/9] Show that a Columns menu change was not saved when the save address answers with an error Co-Authored-By: Claude Opus 5.5 --- .../keystone_ui/column_picker_controller.js | 10 ++++-- .../ui/column_picker_component.html.erb | 1 + .../keystone/ui/column_picker_component.rb | 1 + .../column_picker_controller_test.js | 32 +++++++++++++++++-- 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/keystone_ui/column_picker_controller.js b/app/assets/javascripts/keystone_ui/column_picker_controller.js index 57bc30de..3bea6e04 100644 --- a/app/assets/javascripts/keystone_ui/column_picker_controller.js +++ b/app/assets/javascripts/keystone_ui/column_picker_controller.js @@ -1,7 +1,7 @@ import { Controller } from "@hotwired/stimulus" export default class extends Controller { - static targets = ["menu", "option"] + static targets = ["menu", "option", "error"] static values = { saveUrl: String } connect() { @@ -99,8 +99,14 @@ 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" }) }) } + + failed() { + this.errorTarget.classList.remove("hidden") + } } diff --git a/app/components/keystone/ui/column_picker_component.html.erb b/app/components/keystone/ui/column_picker_component.html.erb index b784abbf..396de0e7 100644 --- a/app/components/keystone/ui/column_picker_component.html.erb +++ b/app/components/keystone/ui/column_picker_component.html.erb @@ -29,4 +29,5 @@ <% end %> + diff --git a/app/components/keystone/ui/column_picker_component.rb b/app/components/keystone/ui/column_picker_component.rb index 8b57f9db..398d57b6 100644 --- a/app/components/keystone/ui/column_picker_component.rb +++ b/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 diff --git a/test/javascript/column_picker_controller_test.js b/test/javascript/column_picker_controller_test.js index 36033bb5..a08c869b 100644 --- a/test/javascript/column_picker_controller_test.js +++ b/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,29 @@ 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 } + 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.mark(on(1)) + controller.close(outside) + }) + + assert.equal(error.classList.contains("hidden"), false) +}) From 907101c8072deb3c03da611f8cc7cf6dd4c4e55c Mon Sep 17 00:00:00 2001 From: tylercschneider Date: Wed, 7 Oct 2026 10:27:33 -0400 Subject: [PATCH 2/9] Show that a Columns menu change was not saved when the save cannot reach the server Co-Authored-By: Claude Opus 5.5 --- .../keystone_ui/column_picker_controller.js | 2 +- test/javascript/column_picker_controller_test.js | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/keystone_ui/column_picker_controller.js b/app/assets/javascripts/keystone_ui/column_picker_controller.js index 3bea6e04..a8ac248f 100644 --- a/app/assets/javascripts/keystone_ui/column_picker_controller.js +++ b/app/assets/javascripts/keystone_ui/column_picker_controller.js @@ -103,7 +103,7 @@ export default class extends Controller { if (!response.ok) return this.failed() Turbo.visit(window.location.href, { action: "replace" }) - }) + }).catch(() => this.failed()) } failed() { diff --git a/test/javascript/column_picker_controller_test.js b/test/javascript/column_picker_controller_test.js index a08c869b..75f9ba1d 100644 --- a/test/javascript/column_picker_controller_test.js +++ b/test/javascript/column_picker_controller_test.js @@ -233,3 +233,18 @@ test("a save the server answers with an error shows that the change was not save 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.mark(on(1)) + controller.close(outside) + }) + + assert.equal(error.classList.contains("hidden"), false) +}) From 39ef904c8c26ad36d1a111d90ddc0659b9e0bb2f Mon Sep 17 00:00:00 2001 From: tylercschneider Date: Wed, 7 Oct 2026 10:27:41 -0400 Subject: [PATCH 3/9] Pin that a failed Columns menu save leaves the page without reloading it Co-Authored-By: Claude Opus 5.5 --- test/javascript/column_picker_controller_test.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/javascript/column_picker_controller_test.js b/test/javascript/column_picker_controller_test.js index 75f9ba1d..29f959f8 100644 --- a/test/javascript/column_picker_controller_test.js +++ b/test/javascript/column_picker_controller_test.js @@ -248,3 +248,18 @@ test("a save that cannot reach the server shows that the change was not saved", 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.mark(on(1)) + controller.close(outside) + }) + + assert.deepEqual(visits, []) +}) From 5f967b490ffd21954bf43b67555371a2c10abb7c Mon Sep 17 00:00:00 2001 From: tylercschneider Date: Wed, 7 Oct 2026 10:27:52 -0400 Subject: [PATCH 4/9] Put a Columns menu's boxes back to what the table shows after a failed save Co-Authored-By: Claude Opus 5.5 --- .../keystone_ui/column_picker_controller.js | 5 +++++ .../column_picker_controller_test.js | 18 +++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/keystone_ui/column_picker_controller.js b/app/assets/javascripts/keystone_ui/column_picker_controller.js index a8ac248f..b4082607 100644 --- a/app/assets/javascripts/keystone_ui/column_picker_controller.js +++ b/app/assets/javascripts/keystone_ui/column_picker_controller.js @@ -5,6 +5,7 @@ export default class extends Controller { 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) } @@ -108,5 +109,9 @@ export default class extends Controller { failed() { this.errorTarget.classList.remove("hidden") + this.shown.forEach(([ option, checked ]) => { + this.checkboxIn(option).checked = checked + option.querySelector("label").classList.toggle("ks-menu-option-hidden", !checked) + }) } } diff --git a/test/javascript/column_picker_controller_test.js b/test/javascript/column_picker_controller_test.js index 29f959f8..f3c26326 100644 --- a/test/javascript/column_picker_controller_test.js +++ b/test/javascript/column_picker_controller_test.js @@ -210,7 +210,7 @@ test("closing the menu with its Columns button after a tick sends the hidden col async function afterSaving(answer, run) { const visits = [] - globalThis.document = { querySelector: () => null } + globalThis.document = { querySelector: () => null, addEventListener() {}, removeEventListener() {} } globalThis.Turbo = { visit: (url) => visits.push(url) } globalThis.window = { location: { href: "/months" } } globalThis.fetch = () => answer() @@ -263,3 +263,19 @@ test("a failed save leaves the page without reloading it", async () => { 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 ] ]) +}) From 2d5acf0bb3ad2014b5b4e9b23593f4b286720548 Mon Sep 17 00:00:00 2001 From: tylercschneider Date: Wed, 7 Oct 2026 10:28:07 -0400 Subject: [PATCH 5/9] Connect the Columns menu in the failed-save tests as the page does, so they pass with its starting state recorded Co-Authored-By: Claude Opus 5.5 --- test/javascript/column_picker_controller_test.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/javascript/column_picker_controller_test.js b/test/javascript/column_picker_controller_test.js index f3c26326..2b4ba9c4 100644 --- a/test/javascript/column_picker_controller_test.js +++ b/test/javascript/column_picker_controller_test.js @@ -227,6 +227,7 @@ test("a save the server answers with an error shows that the change was not save options[1].parts.checkbox.checked = false await afterSaving(() => Promise.resolve({ ok: false }), () => { + controller.connect() controller.mark(on(1)) controller.close(outside) }) @@ -242,6 +243,7 @@ test("a save that cannot reach the server shows that the change was not saved", options[1].parts.checkbox.checked = false await afterSaving(() => Promise.reject(new TypeError("Failed to fetch")), () => { + controller.connect() controller.mark(on(1)) controller.close(outside) }) @@ -257,6 +259,7 @@ test("a failed save leaves the page without reloading it", async () => { options[1].parts.checkbox.checked = false const visits = await afterSaving(() => Promise.resolve({ ok: false }), () => { + controller.connect() controller.mark(on(1)) controller.close(outside) }) From 572facdafa68363d1d8961f2f1b3b06b4bd11d48 Mon Sep 17 00:00:00 2001 From: tylercschneider Date: Wed, 7 Oct 2026 10:28:17 -0400 Subject: [PATCH 6/9] Put a Columns menu's order back to what the table shows after a failed save Co-Authored-By: Claude Opus 5.5 --- .../keystone_ui/column_picker_controller.js | 6 ++++++ test/javascript/column_picker_controller_test.js | 15 +++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/app/assets/javascripts/keystone_ui/column_picker_controller.js b/app/assets/javascripts/keystone_ui/column_picker_controller.js index b4082607..2aa957a2 100644 --- a/app/assets/javascripts/keystone_ui/column_picker_controller.js +++ b/app/assets/javascripts/keystone_ui/column_picker_controller.js @@ -67,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 @@ -110,8 +114,10 @@ export default class extends Controller { 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() } } diff --git a/test/javascript/column_picker_controller_test.js b/test/javascript/column_picker_controller_test.js index 2b4ba9c4..0164f9de 100644 --- a/test/javascript/column_picker_controller_test.js +++ b/test/javascript/column_picker_controller_test.js @@ -282,3 +282,18 @@ test("after a failed save the menu's boxes go back to what the table shows", asy 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" ]) +}) From c248bfe2a599dffa89c67fd5ed97cfa12cf1f3c2 Mon Sep 17 00:00:00 2001 From: tylercschneider Date: Wed, 7 Oct 2026 10:28:36 -0400 Subject: [PATCH 7/9] Pin that a data table's Columns menu holds a hidden message for a change that was not saved Co-Authored-By: Claude Opus 5.5 --- test/render/data_table_component_render_test.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/render/data_table_component_render_test.rb b/test/render/data_table_component_render_test.rb index 62e08b19..3b276af0 100644 --- a/test/render/data_table_component_render_test.rb +++ b/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 From f086b8d050a6784e7eb7bfce5e0206f52f191414 Mon Sep 17 00:00:00 2001 From: tylercschneider Date: Wed, 7 Oct 2026 10:28:44 -0400 Subject: [PATCH 8/9] Describe the Columns menu's message for a change that was not saved in the readme and changelog Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 +++ README.md | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 24be3368..a7475f86 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/README.md b/README.md index 61f1ad04..1c6bb193 100644 --- a/README.md +++ b/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( From f975f6736a7cd19e6a536d9ed0586b4ca2070b22 Mon Sep 17 00:00:00 2001 From: tylercschneider Date: Wed, 7 Oct 2026 10:31:13 -0400 Subject: [PATCH 9/9] Describe the Columns menu's message for a change that was not saved in keystone_ui's locals Co-Authored-By: Claude Opus 5.5 --- the_local/agents/keystone_ui-develop.md | 11 ++++++++--- the_local/agents/keystone_ui-info.md | 5 ++++- the_local/agents/keystone_ui-install.md | 5 +++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/the_local/agents/keystone_ui-develop.md b/the_local/agents/keystone_ui-develop.md index 7c0d0459..9f3c814f 100644 --- a/the_local/agents/keystone_ui-develop.md +++ b/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 diff --git a/the_local/agents/keystone_ui-info.md b/the_local/agents/keystone_ui-info.md index 18ee7c58..36b1592e 100644 --- a/the_local/agents/keystone_ui-info.md +++ b/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 diff --git a/the_local/agents/keystone_ui-install.md b/the_local/agents/keystone_ui-install.md index 1a55967d..959b9387 100644 --- a/the_local/agents/keystone_ui-install.md +++ b/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.