From af5a71347b955c3a9ba83eaceb86bcdc5a18066a Mon Sep 17 00:00:00 2001 From: Ryan Hughes Date: Wed, 13 May 2026 10:34:47 -0400 Subject: [PATCH] Address reviewer findings for bar settings GUI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CRITICAL: - Re-inject working entry into the settings form every time the dialog opens. Previously the Loader's onLoaded only fires when the sourceComponent identity changes, so reopening the dialog on the same widget id (or on a second instance of the same id, e.g. two spacers) showed stale field values. - 'Add widget' menu and the centerAnchor dropdown both read draft.layout[section] without depending on the section array identity. Mutating the array in place did not invalidate those bindings, so the menu would show a stale availability list and let non-spacer widgets be added twice. Layout mutations now replace the whole object so any binding that reads it re-evaluates. WARNINGS: - Save now writes only the diff against the merged defaults so future bar-defaults changes are not silently shadowed by a stale snapshot. - FileView onFileChanged handlers simplified to a single reload() — the explicit second loader call was duplicating loadConfig and bumping barConfigSerial twice per change (visible flicker). - userFile onLoaded ignores reloads while the user has an unsaved working draft so external edits during editing do not clobber the in-progress state. - SpinBox forms switched from onValueChanged to onValueModified so the initial-value bind no longer fires fieldChanged. - Hyprland window-rule sizes aligned with the FloatingWindow implicitWidth/Height values (720 main, 380 dialog). - omarchy-launch-bar-settings falls back to initialTitle when focusing, so a dialog that has not yet had its title applied still surfaces the existing instance. --- bin/omarchy-launch-bar-settings | 4 +- default/hypr/apps/omarchy-bar-settings.lua | 4 +- default/quickshell/bar-settings/shell.qml | 95 ++++++++++++++++------ default/quickshell/bar/shell.qml | 15 +--- 4 files changed, 76 insertions(+), 42 deletions(-) diff --git a/bin/omarchy-launch-bar-settings b/bin/omarchy-launch-bar-settings index 178db271..531d803b 100755 --- a/bin/omarchy-launch-bar-settings +++ b/bin/omarchy-launch-bar-settings @@ -9,7 +9,9 @@ OMARCHY_PATH="${OMARCHY_PATH:-$HOME/.local/share/omarchy}" CONFIG_DIR="$OMARCHY_PATH/default/quickshell/bar-settings" if quickshell list -p "$CONFIG_DIR" 2>/dev/null | grep -q '^Instance '; then - hyprctl dispatch focuswindow 'title:Omarchy bar settings' >/dev/null 2>&1 + if ! hyprctl dispatch focuswindow 'title:Omarchy bar settings' 2>/dev/null | grep -q ok; then + hyprctl dispatch focuswindow 'initialTitle:Omarchy bar settings' >/dev/null 2>&1 + fi exit 0 fi diff --git a/default/hypr/apps/omarchy-bar-settings.lua b/default/hypr/apps/omarchy-bar-settings.lua index e8247838..22728d23 100644 --- a/default/hypr/apps/omarchy-bar-settings.lua +++ b/default/hypr/apps/omarchy-bar-settings.lua @@ -1,4 +1,4 @@ hl.window_rule({ match = { title = "^Omarchy bar settings$" }, tag = "+floating-window" }) -hl.window_rule({ match = { title = "^Omarchy bar settings$" }, size = { 760, 760 } }) +hl.window_rule({ match = { title = "^Omarchy bar settings$" }, size = { 720, 720 } }) hl.window_rule({ match = { title = "^Widget settings " }, tag = "+floating-window" }) -hl.window_rule({ match = { title = "^Widget settings " }, size = { 420, 360 } }) +hl.window_rule({ match = { title = "^Widget settings " }, size = { 380, 320 } }) diff --git a/default/quickshell/bar-settings/shell.qml b/default/quickshell/bar-settings/shell.qml index 3ed25d1a..7cc46ad3 100644 --- a/default/quickshell/bar-settings/shell.qml +++ b/default/quickshell/bar-settings/shell.qml @@ -91,9 +91,38 @@ ShellRoot { draftRevision++ } + function deepEqual(a, b) { + return JSON.stringify(a) === JSON.stringify(b) + } + + function diffAgainstDefaults() { + var defaults = mergeConfig( + { position: "top", centerAnchor: "", fontFamily: "JetBrainsMono Nerd Font", layout: { left: [], center: [], right: [] } }, + defaultConfig + ) + var override = {} + if (!deepEqual(draft.position, defaults.position)) override.position = draft.position + if (!deepEqual(draft.centerAnchor, defaults.centerAnchor)) override.centerAnchor = draft.centerAnchor + if (!deepEqual(draft.fontFamily, defaults.fontFamily)) override.fontFamily = draft.fontFamily + + var defaultLayout = normalizeLayout(defaults.layout || {}) + var layoutDiff = {} + var hasLayoutDiff = false + var sections = ["left", "center", "right"] + for (var i = 0; i < sections.length; i++) { + var s = sections[i] + if (!deepEqual(draft.layout[s], defaultLayout[s])) { + layoutDiff[s] = draft.layout[s] + hasLayoutDiff = true + } + } + if (hasLayoutDiff) override.layout = layoutDiff + return override + } + function saveConfig() { - var payload = cloneJson(draft) - userFile.setText(JSON.stringify(payload, null, 2) + "\n") + var override = diffAgainstDefaults() + userFile.setText(JSON.stringify(override, null, 2) + "\n") dirty = false } @@ -107,35 +136,44 @@ ShellRoot { draftRevision++ } - function moveEntry(section, fromIndex, toIndex) { - var arr = draft.layout[section].slice() - if (toIndex < 0 || toIndex >= arr.length) return - var item = arr[fromIndex] - arr.splice(fromIndex, 1) - arr.splice(toIndex, 0, item) - draft.layout[section] = arr + // Replace the whole `layout` object so any binding that reads `draft.layout` + // is invalidated. Mutating `draft.layout[section]` alone does not notify QML. + function mutateLayout(section, mutator) { + var nextLayout = { + left: draft.layout.left.slice(), + center: draft.layout.center.slice(), + right: draft.layout.right.slice() + } + mutator(nextLayout[section]) + var nextDraft = { + position: draft.position, + centerAnchor: draft.centerAnchor, + fontFamily: draft.fontFamily, + layout: nextLayout + } + draft = nextDraft markDirty() } + function moveEntry(section, fromIndex, toIndex) { + if (toIndex < 0 || toIndex >= draft.layout[section].length) return + mutateLayout(section, function(arr) { + var item = arr[fromIndex] + arr.splice(fromIndex, 1) + arr.splice(toIndex, 0, item) + }) + } + function removeEntry(section, index) { - var arr = draft.layout[section].slice() - arr.splice(index, 1) - draft.layout[section] = arr - markDirty() + mutateLayout(section, function(arr) { arr.splice(index, 1) }) } function addEntry(section, id) { - var arr = draft.layout[section].slice() - arr.push({ id: id }) - draft.layout[section] = arr - markDirty() + mutateLayout(section, function(arr) { arr.push({ id: id }) }) } function updateEntry(section, index, newEntry) { - var arr = draft.layout[section].slice() - arr[index] = cloneJson(newEntry) - draft.layout[section] = arr - markDirty() + mutateLayout(section, function(arr) { arr[index] = cloneJson(newEntry) }) } function loadTheme(raw) { @@ -235,7 +273,7 @@ ShellRoot { watchChanges: true printErrors: false onLoaded: root.loadConfig() - onFileChanged: { reload(); root.loadConfig() } + onFileChanged: reload() } FileView { @@ -244,8 +282,11 @@ ShellRoot { watchChanges: true atomicWrites: true printErrors: false - onLoaded: root.loadConfig() - onFileChanged: { reload(); root.loadConfig() } + onLoaded: { + if (root.dirty) return + root.loadConfig() + } + onFileChanged: reload() } FileView { @@ -253,7 +294,7 @@ ShellRoot { watchChanges: true printErrors: false onLoaded: root.loadTheme(text()) - onFileChanged: { reload(); root.loadTheme(text()) } + onFileChanged: reload() } FloatingWindow { @@ -794,7 +835,7 @@ ShellRoot { from: 0 to: 256 value: spacerForm.entry.size !== undefined ? spacerForm.entry.size : 12 - onValueChanged: spacerForm.fieldChanged("size", value) + onValueModified: spacerForm.fieldChanged("size", value) } } } @@ -875,7 +916,7 @@ ShellRoot { from: 1 to: 25 value: brightForm.entry.step !== undefined ? brightForm.entry.step : 5 - onValueChanged: brightForm.fieldChanged("step", value) + onValueModified: brightForm.fieldChanged("step", value) } } } diff --git a/default/quickshell/bar/shell.qml b/default/quickshell/bar/shell.qml index 4d6cb690..ecf0ef1b 100644 --- a/default/quickshell/bar/shell.qml +++ b/default/quickshell/bar/shell.qml @@ -676,10 +676,7 @@ ShellRoot { watchChanges: true printErrors: false onLoaded: root.loadDefaultBarConfig(text()) - onFileChanged: { - reload() - root.loadDefaultBarConfig(text()) - } + onFileChanged: reload() } FileView { @@ -687,10 +684,7 @@ ShellRoot { watchChanges: true printErrors: false onLoaded: root.loadUserBarConfig(text()) - onFileChanged: { - reload() - root.loadUserBarConfig(text()) - } + onFileChanged: reload() } FileView { @@ -698,10 +692,7 @@ ShellRoot { watchChanges: true printErrors: false onLoaded: root.loadTheme(text()) - onFileChanged: { - reload() - root.loadTheme(text()) - } + onFileChanged: reload() } Process {