Waybar icon indicating copy to clipboard operation
Waybar copied to clipboard

Sway window newstyles

Open RobertMueller2 opened this issue 2 years ago • 16 comments

A more sophisticated implementation for issue 1399, following the suggestions from https://github.com/Alexays/Waybar/issues/1399#issuecomment-1024913010

  • Might break existing styles, but only if people relied on floating nodes being counted (introduced in commit 99723845976fb577a057b86e36b502c966bf254f -- so basically kinda breaks again what we fixed in that recent commit, but by using the new floating style, there should be a suitable workaround.)
  • Provides classes empty, floating, tabbed, tiled, solo, stacked, and app_id. Some trace log output.
  • Adds an "offscreen-css" bool option (default false), only effective when "all-outputs" is set. This provides styling according to windows on the given output when it's not focused, rather than showing the focused screens focused window and style.
  • Adds an "offscreen-css-text" string option (default empty), only effective when "all-ouputs" and "offscreen-style" are set. This is shown as a text on the unfocused output.
  • Adds a "show-focused-workspace" bool option (default false) to indicate the workspace name if the whole workspace is focused if nodes are present. If not set, empty text is shown but css classes according to nodes in the workspace are still applied.

Limitations:

  • when the top-level layout changes (e.g. tabbed to stacked), there is no sway event (well, there might be a keybinding event, but that is too weak) therefore the implementation cannot react here. The only way around I could think of would be to poll get_tree on a timer. Asking that sway fire an event for this is unlikely, considering that i3 doesn't do it either. I think I tested that. If I remember correctly.
  • I think the recursion could still be improved, getFocusedNode already iterates all leaves, and then leafNodesInWorkspace does it again for at least one workspace per output. It did not come across as a problem during testing, but I think the code could be more concise. On the other hand, the given change is already big enough as it is, but I have as much as I think was possible tried to stay within the existing implementation. I wouldn't exactly feel comfortable with almost rewriting the whole thing. ;)
  • while we're at it, I think the active workspace as well as the focused window could be passed as Json::Value instead of string content, then the update function can handle how the info is displayed, this could allow for different replacements for format, like marks, idle_inhibition status...

Review questions:

  • I'm not sure if the case for immediateParent is good. It looks like parentWorkspace (which existed before), but I'm unsure if that's correct according to the style guideline.

Still to do:

  • [x] FIXME: app_id should only be set when it's the only window. manpage says so, so keep that behaviour.
  • [x] FIXME: there's still a problem with floating windows
  • [x] Optimise code here and there
  • ~~make output/workspace filtering optional~~ EDIT: Don't think this makes sense.
  • [x] use clang-format on each of the commits to allow rebase again
  • [x] consider "offscreen" style vs offscreen window situation, based on option
  • [x] revisit code style
  • ~~perhaps add "floating"/"tiled" styles depending on what the focused window is~~ EDIT: leaving this for a future feature
  • [x] Man page update
  • [x] ~~show-focused-workspace still isn't correct~~
  • [ ] More testing after rebasing

Tested with this style:

window#waybar {
  background-color: #990000;
}

window#waybar.empty {
  background-color: transparent;
}

window#waybar.empty #window {
  padding: 0px;
  margin: 0px;
  border: 0px;
/*  background-color: rgba(66,66,66,0.5); */ /* transparent */
  background-color: transparent;
}

window#waybar.solo #window {
    padding-left: 5px;
    padding-right: 5px;
    color: #eee8d5; /* base2 */
    background-color: #073642; /*base02*/
}

window#waybar.floating #window {
    padding-left: 5px;
    padding-right: 5px;
    color: #eee8d5; /* base2 */
    background-color: #b58900; /*yellow*/
}

window#waybar.tiled #window {
    padding-left: 5px;
    padding-right: 5px;
    color: #eee8d5; /* base2 */
    background-color: #cb4b16; /* orange */

}
window#waybar.stacked #window {
    padding-left: 5px;
    padding-right: 5px;
    color: #eee8d5; /* base2 */
    background: #2aa196; /*cyan*/
}
window#waybar.tabbed #window {
    padding-left: 5px;
    padding-right: 5px;
    color: #eee8d5; /* base2 */
    background: #859900; /*green*/
}

window#waybar.code {
    background-color: #007ACC;
}

and this config (testing all combinations of all-output/offscreen-style, offscreen-text set or not set):

{
    "layer": "top", // Waybar at top layer
    "position": "top", // Waybar position (top|bottom|left|right)
    "height": 39, // Waybar height (to be removed for auto height)
    "modules-center": ["sway/window", "custom/media"],
    "sway/window": {
        "format": "{}",
        "all-outputs" : true,
        "offscreen-css" : true,
        "offscreen-css-text": "unfocused",
        "show-focused-workspace-name" : true,
        "max-length": 50,
        "icon": true,
        "rewrite": {
            "(.*) - Mozilla Firefox": " $1",
            "fish (.*)": "> [$1]"
        },
}

RobertMueller2 avatar Feb 02 '22 18:02 RobertMueller2

Something I noticed, when switching the workspace layout (e.g. tabbed to splith), there is no immediate class change. Only after focus change.

I'm not exactly sure how we can deal with that, because the only event I saw was for the keybinding.

RobertMueller2 avatar Feb 02 '22 19:02 RobertMueller2

This seems that can be a solution to my issue here too #1349 However I can't get that patch to work. I have added the classes described above but it does not seems to change anything

primalmotion avatar Feb 07 '22 22:02 primalmotion

I think your use case should even work before the patch, I'll add a comment in #1349.

Also I'm wondering if it helps for easier to understand styling if we also add classes to the label, not just the whole bar.

But I'm not sure why the patch doesn't do anything for you. Do you see any trace output from the module?

RobertMueller2 avatar Feb 08 '22 08:02 RobertMueller2

As stated in the #1349, my issue was that I did not use the sway-window module. it works when I enable it. sorry for the noise :)

primalmotion avatar Feb 08 '22 17:02 primalmotion

As stated in the #1349, my issue was that I did not use the sway-window module. it works when I enable it. sorry for the noise :)

no worries :)

RobertMueller2 avatar Feb 08 '22 20:02 RobertMueller2

~~Moved left TODOs to first post.~~

RobertMueller2 avatar Feb 19 '22 12:02 RobertMueller2

~~Just noticed that not all nodes necessarily have the output property set. That requires more changes.~~

RobertMueller2 avatar Feb 25 '22 15:02 RobertMueller2

~~Sadly, the current approach fails with several tiled windows, at least when using the offscreen-text option. I was hoping not to change the underlying logic too much, but now I think it'll be more work. Could take a bit, too.~~

Edit: Actually, found a way to fix it. I hope anyway. :P

RobertMueller2 avatar Feb 27 '22 08:02 RobertMueller2

I think I've done all I can here. I'll test the latest for a few more days and then remove the draft flag.

RobertMueller2 avatar Mar 20 '22 09:03 RobertMueller2

~~While using this PR, I've noticed some behaviour which seems to be related to external screen connection, disconnection and reconnection. The display of the focused window gets "stuck" when changing screen focus, because there is some node that is being iterated where there's no Type child node and/or string comparison is not possible.~~

~~I'm not sure if I can reproduce it just like that, but I feel I need to investigate further what went wrong, which, unfortunately, delays the PR even more. (And if possible I'd like to find out if the main branch already has the same issue.)~~

~~Perhaps this is an issue in the get_tree, but even if it is, the module needs to be able to deal with it.~~

EDIT: With a combination of mobile try/catch blocks and gdb, I think I might have fixed it, need to observe some more.

RobertMueller2 avatar Mar 24 '22 06:03 RobertMueller2

~~I've had a look at the failing FreeBSD check and I'm not sure why it fails. Looking at the logs, it seems to be a timeout while installing packages rather than actual building. I've setup a FreeBSD VM and I can compile and run the test suite without problems.~~

I guess it was temporary.

RobertMueller2 avatar Apr 02 '22 10:04 RobertMueller2

Rebasing is adding more and more pain. :D I don't think I can guarantee the sanity of each individual commit post-rebasing any longer, which defeats the purpose of keeping them, so I decided to squash them.

I'd like to get this out of the way, but unfortunately, it's difficult at the moment to find enough time to properly test this.

RobertMueller2 avatar May 28 '22 07:05 RobertMueller2

I use this patch especially for the .floating and it works as expected for me

primalmotion avatar Aug 13 '22 00:08 primalmotion

Oh noes. conflicts :(

primalmotion avatar Aug 18 '22 19:08 primalmotion

Oh noes. conflicts :(

rebased, at first glance still seems to work.

RobertMueller2 avatar Aug 28 '22 12:08 RobertMueller2

Thanks! Yeah everything I use still seems to work

primalmotion avatar Aug 29 '22 16:08 primalmotion

I've been using this for ages and have not encountered any issues, except one waybar crash in the context of replugging an external screen, where I was unable to replicate, and where I'm not even sure the reason was the module itself.

That said, I'm a bit swamped at this time. It might be a few weeks before I'm able to look into any issues, should any be reported.

RobertMueller2 avatar Nov 28 '22 17:11 RobertMueller2

Can you also update the github wiki? Thanks for this update and your time :)

Alexays avatar Jan 13 '23 12:01 Alexays

Sure, done.

While doing so I've noticed a small error I've added to the manpage, will add a pull request in a min.

RobertMueller2 avatar Jan 13 '23 14:01 RobertMueller2