Skip to content
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

Undefined workspace settings handling #188

Closed
wants to merge 2 commits into from

Conversation

olejorgenb
Copy link
Member

No description provided.

"Unidentified" workspace settings can behave strangely when cycling
settings (and especially if dynamic workspaces is used)
Very confusing when cycling settings when we pick from the Meta prefs.
Color changes, but name stays the same since the index of the workspace isn't
changing.

Cycling the Meta prefs in parallel would likely help, but not worth the
complexity IMO.
@@ -133,6 +149,9 @@ function getNewWorkspaceSettings(index) {
list.push(uuid);
workspaceList.set_strv('list', list);
settings.set_int('index', index);
settings.set_string('color', getUnusedColor());
let n = findWorkspaceSettingsByName(/^Unnamed .*/).length;
settings.set_string(`Unnamed ${n+1}`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there anything which prevents us from using Workspace ${i} here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, as long as the list is "monotonic" (which it is atm.) that would also work

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I'd vote for Workspace n in that case, as that makes it clearer that it's a workspace related.

@smichel17 smichel17 marked this pull request as draft September 7, 2022 21:33
@smichel17 smichel17 mentioned this pull request Sep 7, 2022
@jtaala
Copy link
Collaborator

jtaala commented Sep 4, 2023

Closing this as #584 addresses this issue - as well as work with dynamic workspaces (and injecting workspaces between other workspaces - e.g. non-monotonic workspace naming).

@jtaala jtaala closed this Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants