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

Add "auto unstack" switch #1657

Open
wants to merge 9 commits into
base: master_jammy
Choose a base branch
from

Conversation

MeowcaTheoRange
Copy link

@MeowcaTheoRange MeowcaTheoRange commented Oct 3, 2023

Create a toggle switch in the settings to enable destroying stacks when they have only one window.

This is my first PR, and it "works on my machine" when I run make debug. There still might be bugs, though.

My system specs:

OS: Nobara Linux 37 (Thirty Seven) x86_64 
Kernel: 6.3.5-201.fsync.fc37.x86_64 
Resolution: 1920x1080 
DE: GNOME 43.2 on Mutter X11
WM Theme: Adwaita
Theme: adw-gtk3-dark [GTK2/3] 
Icons: Adwaita [GTK2/3] 
CPU: Intel i7-10750H (12) @ 2.600GHz 
GPU: NVIDIA GeForce GTX 1660 Ti Mobile 
Memory: 7833MiB 

@MeowcaTheoRange
Copy link
Author

MeowcaTheoRange commented Oct 4, 2023

Oh, I should also explain the strange indent and .prettierignore file.
Well actually, I'm pretty sure they explain themselves if you've ever used Prettier.

Well, I fixed it, but Prettier also changed some other things. Will fix soon.

@MeowcaTheoRange
Copy link
Author

image
[ALT: A cake with the text "Sorry I committed code that was formatted by my IDE" "(it might happen again)"]

@jacobgkau jacobgkau requested review from a team October 4, 2023 23:04
@jacobgkau jacobgkau self-assigned this Oct 4, 2023
Copy link
Member

@jacobgkau jacobgkau left a comment

Choose a reason for hiding this comment

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

This doesn't seem to be working. After turning the option on, I can still move windows out of a stack and have a single remaining window remain in the stack, rather than having the stack be destroyed.

simplescreenrecorder-2023-10-12_17.37.42.mp4

The most recent commit also broke the tab bar's formatting, as shown in the above video (the commit message includes do not PR, so I assume pushing it here was a mistake?)

@MeowcaTheoRange
Copy link
Author

MeowcaTheoRange commented Oct 13, 2023

This doesn't seem to be working. After turning the option on, I can still move windows out of a stack and have a single remaining window remain in the stack, rather than having the stack be destroyed.
simplescreenrecorder-2023-10-12_17.37.42.mp4

The most recent commit also broke the tab bar's formatting, as shown in the above video (the commit message includes do not PR, so I assume pushing it here was a mistake?)

Oh no, I didn't mean to PR the recent commit, I think it just did it automatically. I'll try to remove the commit.

Also, it might not be working because my commit might have only affected mouse stacking - I based my code off of the code that reverted this stacking behaviour, so I'm not sure how to also make it apply to keyboard stacking.

@jacobgkau
Copy link
Member

Also, it might not be working because my commit might have only affected mouse stacking - I based my code off of the code that reverted this stacking behaviour, so I'm not sure how to also make it apply to keyboard stacking.

The current wording of the setting:

Destroy a window stack when it contains only one window

...does not reflect that it's specific to the mouse. If you don't want to get it working for the keyboard, then you could alternatively tweak the wording so it's more indicative of what it does, e.g.:

Destroy stacks after removing second-to-last window with mouse

(The wording could be nicer, but I tried to limit the length and also match the wording of existing settings in this panel.)

@MeowcaTheoRange
Copy link
Author

Also, it might not be working because my commit might have only affected mouse stacking - I based my code off of the code that reverted this stacking behaviour, so I'm not sure how to also make it apply to keyboard stacking.

The current wording of the setting:

Destroy a window stack when it contains only one window

...does not reflect that it's specific to the mouse. If you don't want to get it working for the keyboard, then you could alternatively tweak the wording so it's more indicative of what it does, e.g.:

Destroy stacks after removing second-to-last window with mouse

(The wording could be nicer, but I tried to limit the length and also match the wording of existing settings in this panel.)

Well, I do want to accomodate for keyboard stacking, I just don't know how to.

But, if it's just fine with the mouse, it could be called "Destroy stacks when separated with mouse" or something like that.

src/prefs.ts Outdated Show resolved Hide resolved
src/prefs.ts Outdated
xalign: 0.0
})
xalign: 0.0,
});
Copy link
Member

Choose a reason for hiding this comment

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

It's probably best if you go ahead and revert the unnecessary formatting changes.

Copy link
Author

Choose a reason for hiding this comment

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

Reverting these changes is how I found out VS Code has a shortcut specifically for saving without formatting. It turns out even without Prettier it likes to shuffle around the imports :/

See "Sisyphus" for what my editor tried to do to me while changing this
Copy link
Author

Choose a reason for hiding this comment

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

This is me trying to move the imports back to their rightful place

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.

2 participants