Repository navigation
[ryoku] qsbar: per-monitor workspace isolation (fixes #767) #770
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: unstable-dev
Are you sure you want to change the base?
Changes from all commits
5691e26
6f868a3
b354fa0
fc60a1e
7744fcd
4818296
8b1c349
1776900
b753286
538c985
3ce7431
fa291b4
b413662
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2342,6 +2342,8 @@ Item { | |||||||||||||||||
| // ── workspace display style (orthogonal to mode; persisted) ── | ||||||||||||||||||
| // "rings" is the stable cache token for the user-facing Frame style. | ||||||||||||||||||
| property string workspaceStyle: "default" // default, numbers, magic, kanji, rings, aurora | ||||||||||||||||||
| // Workspace monitor isolation (persisted) | ||||||||||||||||||
| property bool isolateMonitors: false | ||||||||||||||||||
| // The marker styles this variant offers (continuous V2 adds Kanji, Frame, | ||||||||||||||||||
| // Aurora; "rings" is the persisted cache token for the Frame style). | ||||||||||||||||||
| readonly property var workspaceStyleOptions: [ | ||||||||||||||||||
|
|
@@ -2713,6 +2715,7 @@ Item { | |||||||||||||||||
| onLauncherLogoIconChanged: if (_widgetsLoaded) saveWidgets() | ||||||||||||||||||
| onWeatherImperialChanged: if (_widgetsLoaded) saveWidgets() | ||||||||||||||||||
| onWorkspaceStyleChanged: if (_widgetsLoaded) saveWidgets() | ||||||||||||||||||
| onIsolateMonitorsChanged: if (_widgetsLoaded) persistWidgetsToConfig() | ||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 minor · Maintainability · fix: low effortThe latest commit subject, 💡 Related suggestion (possible bug)Save isolation through the widget persistence path Suggested change
Suggested change
Reply here to discuss, say it's fixed, or explain why it's intended — no mention needed. |
||||||||||||||||||
| onBarPositionChanged: if (_widgetsLoaded) saveWidgets() | ||||||||||||||||||
| onBarScaleChanged: if (_widgetsLoaded) saveWidgets() | ||||||||||||||||||
| onBarShellStyleChanged: if (_widgetsLoaded) saveWidgets() | ||||||||||||||||||
|
|
@@ -3010,6 +3013,7 @@ Item { | |||||||||||||||||
| q.barPosition = barPosition | ||||||||||||||||||
| q.workspaceMode = workspaceMode | ||||||||||||||||||
| q.workspaceStyle = workspaceStyle | ||||||||||||||||||
| q.isolateMonitors = isolateMonitors | ||||||||||||||||||
| q.pickerStyle = pickerStyle | ||||||||||||||||||
| q.launcherLogoMode = launcherLogoMode | ||||||||||||||||||
| q.aiTools = serializeAiTools() | ||||||||||||||||||
|
|
@@ -3079,6 +3083,7 @@ Item { | |||||||||||||||||
| if (q.barPosition === "top" || q.barPosition === "bottom") barPosition = q.barPosition | ||||||||||||||||||
| if (q.workspaceMode !== undefined) workspaceMode = q.workspaceMode | ||||||||||||||||||
| if (q.workspaceStyle !== undefined) workspaceStyle = q.workspaceStyle | ||||||||||||||||||
| if (q.isolateMonitors !== undefined) isolateMonitors = q.isolateMonitors === true | ||||||||||||||||||
| if (q.pickerStyle !== undefined) pickerStyle = q.pickerStyle | ||||||||||||||||||
| if (q.launcherLogoMode !== undefined) launcherLogoMode = q.launcherLogoMode | ||||||||||||||||||
| if (q.aiTools !== undefined) parseAiTools(q.aiTools) | ||||||||||||||||||
|
|
||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -304,10 +304,37 @@ Item { | |
| } | ||
|
|
||
|
|
||
| // ── PREVIEW ── | ||
| // ── workspaces: ISOLATE ── | ||
|
neur0map marked this conversation as resolved.
Resolved
|
||
| Entrance { | ||
| width: page.colW | ||
| index: 4 | ||
| SettingCard { | ||
| width: page.colW | ||
| title: I18n.tr("ISOLATE") | ||
| kana: "\u5206" | ||
|
|
||
| SettingRow { | ||
| anchors.left: parent.left | ||
| anchors.right: parent.right | ||
| label: I18n.tr("Per-monitor") | ||
| desc: I18n.tr("Each bar shows only the workspaces on its own screen.") | ||
|
Comment on lines
+313
to
+320
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 minor · Data Integrity · fix: medium effortThe new translation keys are not added to the source and English catalogs in this diff. The new settings card therefore lacks catalog entries for its title, label, and description, breaking translated-UI integrity.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. Added missing English catalog entries for "Per-monitor" and "Each bar shows only the workspaces on its own screen." in ryoku/i18n/catalog/en.json. |
||
| source: "shell.json" | ||
| controlWidth: Math.max(120, 62 * 2) | ||
| Seg { | ||
| anchors.right: parent.right | ||
| anchors.verticalCenter: parent.verticalCenter | ||
| options: [I18n.tr("Off"), I18n.tr("On")] | ||
| current: page.root && page.root.isolateMonitors ? I18n.tr("On") : I18n.tr("Off") | ||
| onChose: (key) => { if (page.root) page.root.isolateMonitors = (key === I18n.tr("On")) } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // ── PREVIEW ── | ||
| Entrance { | ||
| width: page.colW | ||
| index: 5 | ||
| SettingCard { | ||
| width: page.colW | ||
| title: I18n.tr("PREVIEW") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| .pragma library | ||
|
|
||
| function activeWorkspace(workspaces, screenName, isolateMonitors, focusedWorkspace) { | ||
| if (!isolateMonitors || !screenName) return focusedWorkspace | ||
| for (var i = 0; i < workspaces.length; i++) { | ||
| var workspace = workspaces[i] | ||
| if (!workspace.special && workspace.active && workspace.output === screenName) | ||
| return workspace | ||
| } | ||
| return null | ||
| } | ||
|
|
||
| function needsWorkspaceData(mode, dynamicModel, isolateMonitors) { | ||
| return dynamicModel || mode === "active" || isolateMonitors | ||
| } | ||
|
|
||
| function workspaceList(workspaces, mode, dynamicModel, screenName, isolateMonitors, active, extraWorkspace) { | ||
| if (dynamicModel) { | ||
| var live = [] | ||
| for (var i = 0; i < workspaces.length; i++) { | ||
| var workspace = workspaces[i] | ||
| if (workspace.special) continue | ||
| if (isolateMonitors && screenName && workspace.output && workspace.output !== screenName) | ||
| continue | ||
| live.push(workspace.name) | ||
| } | ||
| return live | ||
| } | ||
|
|
||
| if (mode === "active") { | ||
| var ids = {} | ||
| for (var j = 0; j < workspaces.length; j++) { | ||
| var current = workspaces[j] | ||
| if (current.special) continue | ||
| if (isolateMonitors && screenName && current.output && current.output !== screenName) | ||
| continue | ||
| ids[current.name] = true | ||
| } | ||
| if (active && !active.special | ||
| && (!isolateMonitors || !screenName || !active.output || active.output === screenName)) | ||
| ids[active.name] = true | ||
| return Object.keys(ids).sort(function(a, b) { return Number(a) - Number(b) }) | ||
| } | ||
|
|
||
| var count = mode === "5" ? 5 : 10 | ||
| var fixed = [] | ||
| for (var k = 1; k <= count; k++) { | ||
| var name = String(k) | ||
| var existing = workspaces.find(function(workspace) { return workspace.name === name }) | ||
| if (isolateMonitors && screenName && existing && existing.output && existing.output !== screenName) | ||
| continue | ||
| fixed.push(name) | ||
| } | ||
| if (extraWorkspace) fixed.push(extraWorkspace) | ||
| return fixed | ||
| } | ||
|
|
||
| function extraWorkspace(focused, mode) { | ||
| var count = mode === "5" ? 5 : 10 | ||
| var number = focused ? Number(focused.name) : 0 | ||
| return number > count ? focused.name : "" | ||
| } | ||
|
|
||
| function hasMonitorFocus(isolateMonitors, screenName, focusedOutput) { | ||
| return !isolateMonitors || !screenName || !focusedOutput || focusedOutput === screenName | ||
| } | ||
|
|
||
| function markerOpacity(hasFocus) { | ||
| return hasFocus ? 1.0 : 0.72 | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| import assert from "node:assert/strict"; | ||
| import { readFile } from "node:fs/promises"; | ||
| import vm from "node:vm"; | ||
|
|
||
| const source = await readFile(new URL("./WorkspaceIsolation.js", import.meta.url), "utf8"); | ||
| const module = { exports: {} }; | ||
| vm.runInNewContext(source.replace(/^\.pragma library\s*/, "") + | ||
| "\nmodule.exports = { activeWorkspace, needsWorkspaceData, workspaceList, extraWorkspace, hasMonitorFocus, markerOpacity };", | ||
| { module, Number, Object }); | ||
| const { activeWorkspace, needsWorkspaceData, workspaceList, extraWorkspace, hasMonitorFocus, markerOpacity } = module.exports; | ||
|
|
||
| const workspaces = [ | ||
| { name: "1", output: "eDP-1", active: true, special: false }, | ||
| { name: "2", output: "DP-1", active: false, special: false }, | ||
| { name: "12", output: "DP-1", active: true, special: false }, | ||
| { name: "special:1", output: "eDP-1", active: false, special: true } | ||
| ]; | ||
|
|
||
| assert.equal(activeWorkspace(workspaces, "eDP-1", true, workspaces[2]), workspaces[0], | ||
| "each output selects its own active workspace"); | ||
| assert.equal(activeWorkspace(workspaces, "DP-1", true, workspaces[0]), workspaces[2], | ||
| "the other output selects its own active workspace"); | ||
| assert.equal(activeWorkspace(workspaces, "eDP-1", false, workspaces[2]), workspaces[2], | ||
| "disabling isolation keeps the globally focused workspace"); | ||
|
|
||
| assert.equal(needsWorkspaceData("5", false, false), false, | ||
| "fixed mode without isolation does not track live workspace data"); | ||
| assert.equal(needsWorkspaceData("5", false, true), true, | ||
| "fixed mode needs live workspace data to filter monitor assignments"); | ||
| assert.equal(needsWorkspaceData("active", false, false), true, | ||
| "active mode needs live workspace data"); | ||
| assert.equal(needsWorkspaceData("5", true, false), true, | ||
| "dynamic mode needs live workspace data"); | ||
|
|
||
| assert.deepEqual(Array.from(workspaceList(workspaces, "5", false, "eDP-1", true, null, "")), | ||
| ["1", "3", "4", "5"], "fixed mode filters assigned workspaces and keeps uncreated slots"); | ||
| assert.deepEqual(Array.from(workspaceList(workspaces, "5", false, "DP-1", true, null, "12")), | ||
| ["2", "3", "4", "5", "12"], "fixed mode includes the local active workspace beyond its range"); | ||
| assert.equal(extraWorkspace({ name: "1" }, "5"), ""); | ||
| assert.equal(extraWorkspace({ name: "3" }, "5"), "", | ||
| "in-range focus changes keep the extra workspace binding unchanged"); | ||
| assert.equal(extraWorkspace({ name: "12" }, "5"), "12", | ||
| "fixed mode adds a workspace beyond its numbered range"); | ||
|
|
||
| let focusedOutput = "DP-1"; | ||
| assert.equal(markerOpacity(hasMonitorFocus(true, "eDP-1", focusedOutput)), 0.72, | ||
| "an active marker dims when another output has focus"); | ||
| focusedOutput = "eDP-1"; | ||
| assert.equal(markerOpacity(hasMonitorFocus(true, "eDP-1", focusedOutput)), 1.0, | ||
| "the active marker brightens when focus moves to its output"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 minor · Tests · fix: medium effort
The new test only checks persistence through the CLI, leaving the workspace filtering and per-monitor active marker untested. Add coverage for multiple outputs, including uncreated fixed-mode workspaces and focus changes.