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

Persistent centered mode (aka "Sticky Centering") #482

Merged
merged 30 commits into from
Mar 16, 2023
Merged

Conversation

jtaala
Copy link
Collaborator

@jtaala jtaala commented Mar 4, 2023

Fixes #332, #299.

So, this PR resurrects and updates the sticky-center branch. This basically continues to center windows (when switching to other windows) if the window was already centered (e.g. use the shortcut to center a window horizontally --> when switching to other windows they will be centered too).

The updates I added disables this behaviour when using the center shortcut again (on a window that is already centered) - in which case it will "uncenter" that window (move it back to the left edge).

See the video below - in the video I switch between "focus" mode and normal mode (see leftmost topbar icon, which is clickable to switch between modes btw). I'm clicking here but you can use a shortcut to switch between modes too:

paperwm-sticky-centering.mp4

First up, I really like this behaviour when I'm more just focusing on one window/task at a time (instead of looking at multiple side-by-side windows). However, I'm setting this as a draft since I'm not entirely happy with it atm - it's a departure from current behaviour which is to center the current window but not enforce centering view when switching, and will likely confuse users when they try it.

I would suggest:

  • having a separate shortcut to disable/enable a "focus center mode"; ✔️ implemented
  • perhaps add an indicator icon (maybe at left-most position in the topbar) showing what mode PaperWM is in (e.g. centering mode or normal mode). Or just have it show up when in centering mode. ✔️ implemented

Feedback please!

NOTE: this PR has been implemented in the PaperWM-redux fork, which you can install if you want this, or any of my other open PRs.

@jtaala jtaala marked this pull request as draft March 4, 2023 05:07
@Lythenas
Copy link
Collaborator

Lythenas commented Mar 4, 2023

I would suggest:

having a separate shortcut to disable/enable a "focus center mode";
perhaps add an indicator icon (maybe at left-most position in the topbar) showing what mode PaperWM is in (e.g. centering mode or normal mode). Or just have it show up when in centering mode.

I would definitively add your suggestions since it is quite different behavior between "center it one time" and "always center the currently focused window". Also maybe add an option to enable it by default, for people that want that.

Since this is also more like a "mode" now (that you can turn on/off). I think the the check isCentered(last, space) does not make that much sense. I think the "centering mode" state makes more sense to have as a variable in space or maybe globally.

@jtaala
Copy link
Collaborator Author

jtaala commented Mar 6, 2023

Hey all, so I've implemented a nice(?) icon/indicator in the very left of the toolbar. You can click it now and it will switch between the modes (normal and centered).

Give it a go and see what you think. I'm liking it thus far.

@jtaala
Copy link
Collaborator Author

jtaala commented Mar 6, 2023

I also added the shortcut super+Shift+c to toggle modes.

@jtaala jtaala marked this pull request as ready for review March 7, 2023 12:01
@jtaala jtaala requested a review from Lythenas March 7, 2023 12:01
@jtaala jtaala mentioned this pull request Mar 11, 2023
@jtaala jtaala linked an issue Mar 11, 2023 that may be closed by this pull request
@Lythenas
Copy link
Collaborator

I will start testing this for the next couple of days.

A couple of things I have noticed on first try:

  • Is the icon on the secondary monitor supposed to be clickable? Because it isn't. If not it's probably fine, the workspace name is also not clickable.
  • Do we want to un-center a window if we disable sticky centering?
  • Can we add a tooltip to the icon/button? E.g. say "Sticky center" or "Normal mode".

@jtaala
Copy link
Collaborator Author

jtaala commented Mar 11, 2023

Thanks @Lythenas for testing, always apprecaited! I really like the behaviour of this and use it quite a bit now.

  • Is the icon on the secondary monitor supposed to be clickable? Because it isn't. If not it's probably fine, the workspace name is also not clickable.

Ah, yeah - so they're not actually real buttons - basically they're just for display. If you switch on Make topbar follow monitor focus then they would work - since they would be the actual buttons on the topbar. Having said that - there's really nothing stopping us from making them "real" - will have a look at it. Icons on other monitors are now clickable ✔️

The shortcut to toggle modes will work though on other monitors though.

Do we want to un-center a window if we disable sticky centering?

I chose not too do that - it's gets a bit difficult since we'd likely want to track which direction is "centered" from - and that adds a little more complexity. I suppose we could store a variable for x position before centering and then return the selected window back to that position on uncentering? will give this a go and see how it works - cheers! un-centering now implemented ✔️

Can we add a tooltip to the icon/button? E.g. say "Sticky center" or "Normal mode".

Likely could - I haven't had the best luck with tooltips (I don't like the way it centers text - but that's mostly for multi line text). Would happily accept a PR on that branch if anyone wants to give it a go. I'll also give this a go and shout out if run into trouble on it.

@Lythenas
Copy link
Collaborator

Lythenas commented Mar 11, 2023

Can we add a tooltip to the icon/button? E.g. say "Sticky center" or "Normal mode".

Likely could - I haven't had the best luck with tooltips (I don't like the way it centers text - but that's mostly for multi line text). Would happily accept a PR on that branch if anyone wants to give it a go. I'll also give this a go and shout out if run into trouble on it.

If it is hard to implement, it might not be worth it. I just thought the icon is probably not obvious.

Do we want to un-center a window if we disable sticky centering?

I chose not too do that - it's gets a bit difficult since we'd likely want to track which direction is "centered" from - and that adds a little more complexity. I suppose we could store a variable for x position before centering and then return the selected window back to that position on uncentering? will give this a go and see how it works - cheers!

I was thinking more like: If there is now empty space on the right/left when uncentering, move the window to the edge of the screen. I don't know how well remembering the original x position works. Because windows could be added/remove/moved since enabling the mode.

But also you don't have to spent that much time adding new features here. I was just posting what came to mind.

@jtaala
Copy link
Collaborator Author

jtaala commented Mar 11, 2023

But also you don't have to spent that much time adding new features here. I was just posting what came to mind.

No, this is appreciated - and I think it's a good idea to see what feels better here. I'll try the x approach (since it's easy) and push it up to see how it feels - note, I'll just literally save the x position and then return whatever window is selected at the time (of undoing focus mode) to that position - might end up feeling weird but will see.

@jtaala
Copy link
Collaborator Author

jtaala commented Mar 13, 2023

The icon is different on Gnome 42 than on Gnome 43.

Oh that's pretty interesting. So, I'm using the preferences-desktop-multitasking-symbolic icon for centering and the sidebar-show-right-symbolic for normal - yeah, I guess other themes versions may have a different icon for that?

Is there anything we can do about that? Maybe ship our own icons? (could just be a copy of the gnome icons)

Sounds like might need to - will have a look at the best way to do that. have now implemented this ✔️

Thanks @Lythenas! Yeah, I'm quite enjoying it - I find myself using it fairly frequently on workspaces with quite disparate tasks (e.g. music, browsing etc.).

can change with gnome ver or theme.

Added improvement to space init - ensures correct layout with
multimonitors and if gnome-shell resetting.
@jtaala
Copy link
Collaborator Author

jtaala commented Mar 13, 2023

@Lythenas, can you test the latest (on Gnome 42 as well)? I'm now packaging the icons in PaperWM so should show icons correctly regardless of gnome version or theme.

@terlar
Copy link

terlar commented Mar 13, 2023

The icons was showing correctly for me on gnome 42, also before this change. So probably some theming issue as suggested

@jtaala
Copy link
Collaborator Author

jtaala commented Mar 14, 2023

  • Can we add a tooltip to the icon/button? E.g. say "Sticky center" or "Normal mode".

Implemented! (well, my version of a tooltip... I forgot how not-great the gtk tooltips are - had trouble with them so just created an implementation myself that I think fits better).

Let me know what you think (I wasn't sure what to put in the tooltip - maybe it's a bit verbose?).

I think we're getting close to completing this feature.

@Lythenas
Copy link
Collaborator

One slight issue with the tooltip: It appears on the wrong monitor :D My topbar is on the right monitor and the tooltip shows on the left monitor. But where I would expect it if I hover over the icon on that monitor. It also doesn't show up if I hover over the icon on my secondary monitor (which is probably also fine, we can improve the behavior if/when we decide how to do topbars on a seconary monitor).

Also maybe make the tooltip background a bit more opaque. It was a little hard to read with my pinned firefox tabs in the background.

Regarding the wrong icons: It could very well be a theming issue (although I think I didn't change anything). But since we are not using the icons for their original purpose I think it is good to bundle them with PaperWM. Just one note on that: Are we allowed to just copy them? What is the license on them? Maybe we need to include a notice somewhere. I would maybe put it either in a comment in the svgs or in a separate file next to the icons.

@jtaala
Copy link
Collaborator Author

jtaala commented Mar 14, 2023

One slight issue with the tooltip: It appears on the wrong monitor :D My topbar is on the right monitor and the tooltip shows on the left monitor. But where I would expect it if I hover over the icon on that monitor.

Oh lol, yes I see - good pickup! (I usually don't use multi monitors - will fix that). FIXED ✔️

It also doesn't show up if I hover over the icon on my secondary monitor (which is probably also fine, we can improve the behavior if/when we decide how to do topbars on a seconary monitor).

Well, given that icon is clickable (even on other monitors) it prob makes sense to have the tooltip show up on that monitor too.

Also maybe make the tooltip background a bit more opaque.

Good call. Have a look at the focus-button-tooltip css class in stylesheet.css - it's set for 0.35 opacity - let me know a good new opacity!

Are we allowed to just copy them? What is the license on them? Maybe we need to include a notice somewhere. I would maybe put it either in a comment in the svgs or in a separate file next to the icons.

Good call - they're from Adwaita theme, and looks like LGPL license? I'll put a note in the resources folder referencing the license - I assume that's all that needs to be done? Anyone have experience here?

@jtaala
Copy link
Collaborator Author

jtaala commented Mar 15, 2023

Yes, we can copy and package the icons - see COPY terms here:

https://github.com/GNOME/adwaita-icon-theme/blob/master/COPYING

I've added an ICONS.info file to resources folder attributing to "Gnome Project" and put some links in there too.

Finally, I've now made the focus icon on secondary (etc.) monitor also show tooltip.

@jtaala
Copy link
Collaborator Author

jtaala commented Mar 15, 2023

Thanks all for testing thus far - please give the latest a test; I believe it's ready!

@jtaala jtaala added the merging soon Label for PRs that are planned to be merged soon (usually within the next week) label Mar 15, 2023
Copy link
Collaborator

@Lythenas Lythenas left a comment

Choose a reason for hiding this comment

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

I think the behavior is now fine.

I will test the tooltip tomorrow and the probably approve.

resources/ICONS.info Outdated Show resolved Hide resolved
jtaala and others added 3 commits March 16, 2023 19:42
@jtaala
Copy link
Collaborator Author

jtaala commented Mar 16, 2023

Oki-doki, updated ICONS.info with reference to original icon filenames used. Also, updated README.md with a small section on focus modes (including hiding the focus-mode button in topbar via dconf if users wish).

Copy link
Collaborator

@Lythenas Lythenas left a comment

Choose a reason for hiding this comment

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

The tooltip works well now.

FYI this is the tooltip definition for Adwaita: https://gitlab.gnome.org/GNOME/libadwaita/-/blob/main/src/stylesheet/widgets/_tooltip.scss They are using 0.8 transparency. But I think it is fine the way it is.

@jtaala jtaala merged commit d5e45a3 into develop Mar 16, 2023
@jtaala jtaala deleted the sticky-center branch March 16, 2023 19:57
jtaala added a commit that referenced this pull request Apr 5, 2023
Fixes #503 

This PR allows users to set the default focus mode (see #482) that is
used when starting a gnome-session (with PaperWM enabled).

This was meant to be part of #482 but I completely forgot about it
:disappointed:. From the `README.md` of this PR:

> ### Setting the default focus mode
> 
> The default focus mode is the standard PaperWM focus mode (i.e. not
centered). This can be changed according to preference by setting the
`default-focus-mode` setting via `dconf` or `gsettings`.
> 
> To set the default focus mode to `CENTER`, execute the following from
a terminal:
> ```
> dconf write /org/gnome/shell/extensions/paperwm/default-focus-mode 1
> ```
> 
> To undo to, or revert to the original PaperWM behaviour (by default),
execute the following:
> ```
> dconf write /org/gnome/shell/extensions/paperwm/default-focus-mode 0
> ```
> 
> _Note: changing this setting during a PaperWM session will set all
spaces to the new default focus mode._

This PR also fixes a few minor issues/annoyances, such as aligning with
`id` name for `show-window-position-bar` schema property, and correctly
showing enabled options in the `Settings ui` (e.g. they can show enabled
but the slider is in the "off" position while it's background shows it's
on).

_NOTE: this PR has been implemented in the
[PaperWM-redux](https://github.com/PaperWM-redux/PaperWM) fork, which
you can install if you want this, or any of [my
PRs](https://github.com/paperwm/PaperWM/pulls/jtaala) that are open._
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merging soon Label for PRs that are planned to be merged soon (usually within the next week)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option to center a window in focus. Always Center
5 participants