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

Implement 'active_only' option and 'visible' class in hyprland/workspaces #2408

Merged
merged 1 commit into from
Sep 5, 2023

Conversation

zjeffer
Copy link
Contributor

@zjeffer zjeffer commented Aug 15, 2023

With this PR, active_only can be set to true, which will only show the active workspace.

If I understand the feature request correctly, each monitor should show the currently visible workspace, right? This PR currently only shows the active one as reported by Hyprland, so if a monitor is not focused, it will not show any workspaces. Is this supposed to happen? I would expect it to be better if each monitor shows one "active" workspace, i.e. the one currently visible.

Let me know if this can be improved.

CC @Anakael @MightyPlaza

@@ -319,6 +330,12 @@ void add_or_remove_class(const Glib::RefPtr<Gtk::StyleContext> &context, bool co
}

void Workspace::update(const std::string &format, const std::string &icon) {
if (this->workspace_manager_.active_only() && !this->active()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once urgent workspaces are implemented in this module, we can simply add a check for this->urgent() here. Then it would work the same as in the wlr/workspaces module.

@MightyPlaza
Copy link
Contributor

i think it should show the current visible workspace in the bar's monitor or the active globally in case of all-outputs option is used

persistent workspaces probably shouldn't be affected by this option since they can be set by the user in the config

(if possible test format-icons with persistent and active workspaces, I haven't tested it yet, but some guys were saying it wasn't working)

Copy link
Contributor

@MightyPlaza MightyPlaza left a comment

Choose a reason for hiding this comment

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

the rest LGTM

@zjeffer
Copy link
Contributor Author

zjeffer commented Aug 15, 2023

(if possible test format-icons with persistent and active workspaces, I haven't tested it yet, but some guys were saying it wasn't working)

What would be the preferred behaviour? If a persistent workspace is active, should it show the active icon (current behaviour) or the persistent icon? The current behaviour seems to work fine for me.

@MightyPlaza
Copy link
Contributor

"hyprland/workspaces": {
    "persistent_workspaces": {
        //"1":[],"2":[],"3":[],"4":[],"5":[],"6":[],"7":[],"8":[],"9":[]
        "*": 10
    },
    "disable-scroll": false,
    "all-outputs": false,
    "warp-on-scroll": false,
    "on-click": "activate",
    "format": "{icon}",
    "format-icons": {
        "1":"一","2":"二","3":"三","4":"四","5":"五","6":"六","7":"七","8":"八","9":"九","10":"十"
    }
},

this config for example
image

was not working as intended, not sure if it has been fixed

@zjeffer
Copy link
Contributor Author

zjeffer commented Aug 15, 2023

Ah, I thought you meant something like this, which does work:

    "format-icons": {
      "default": "O",
      "active": "a",
      "persistent": "p",
      "special": "s"
    },

Your example indeed doesn't work, I'll fix that.

EDIT: Nevermind, it does work on the current master (I was looking at the wrong monitor). I think it may have been fixed here: #2393

@zjeffer zjeffer force-pushed the hyprland/workspaces_active-only branch 2 times, most recently from 4773d1e to f99002f Compare August 23, 2023 17:50
@zjeffer zjeffer changed the title Implement 'active_only' in hyprland/workspaces Implement 'active_only' option and 'visible' class in hyprland/workspaces Aug 29, 2023
@zjeffer zjeffer force-pushed the hyprland/workspaces_active-only branch from f99002f to 9fb7696 Compare September 3, 2023 14:53
@zjeffer zjeffer marked this pull request as ready for review September 3, 2023 16:29
@zjeffer
Copy link
Contributor Author

zjeffer commented Sep 3, 2023

Sorry this took so long, I haven't had much time lately and I kept getting issues compiling due to the fmt & spdlog stuff. Should be ready to review now.

Let me know if this is the correct behaviour or not:

  • Setting both active-only and all-outputs to true => each bar will show all monitor's visible workspaces, and mark the globally active one as active, and the rest as visible.
  • Setting active-only to true and all-outputs to false => each bar shows only its visible workspace. The focused monitor shows the visible workspace as active.
  • Setting active-only to false and all-outputs to true => each bar shows all non-empty workspaces (so each bar shows the exact same thing)
  • Setting both to false => Each bar shows that monitor's non-empty workspaces (default behaviour).
  • Persistent workspaces are not effected, those are always shown.

@zjeffer
Copy link
Contributor Author

zjeffer commented Sep 3, 2023

The visible css class doesn't seem to be working yet for me, but the visible icon does work, investigating...

EDIT: nevermind, I mistyped the css classname in my style.css file (workspace instead of workspaces). Works properly.

@zjeffer zjeffer requested a review from MightyPlaza September 3, 2023 16:47
Copy link
Contributor

@MightyPlaza MightyPlaza left a comment

Choose a reason for hiding this comment

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

lgtm

@khaneliman
Copy link
Contributor

@zjeffer looks like this needs a rebase btw

@zjeffer zjeffer force-pushed the hyprland/workspaces_active-only branch from 30ff369 to 96343c1 Compare September 5, 2023 16:17
@zjeffer
Copy link
Contributor Author

zjeffer commented Sep 5, 2023

@Alexays Ready to merge.

@khaneliman
Copy link
Contributor

Closes #2465

@Alexays Alexays merged commit 80de22a into Alexays:master Sep 5, 2023
@Alexays
Copy link
Owner

Alexays commented Sep 5, 2023

Thanks!

@zjeffer zjeffer deleted the hyprland/workspaces_active-only branch September 5, 2023 21:21
@NotAShelf
Copy link
Contributor

Is it intended behaviour that active-only displays both workspaces if two monitors are at the same time?

@zjeffer
Copy link
Contributor Author

zjeffer commented Sep 8, 2023

@NotAShelf only if all-outputs is true. If it's false, it should only show the current monitor's active workspace.

@NotAShelf
Copy link
Contributor

Understood. Any chance we could get a rule to see one active workspace at a time, regardless of monitor? Similar to active-only + all-outputs behaviour on wlr/workspaces

@zjeffer
Copy link
Contributor Author

zjeffer commented Sep 8, 2023

Do you mean you want to see the same active workspace on every monitor? Like if workspace 1 is active on monitor 1, you also want to see "1" in monitor 2?

@NotAShelf
Copy link
Contributor

NotAShelf commented Sep 8, 2023

More or less, yes. I have a single bar on monitor 1, and I'd love to see only the active workspace. For example if workspace 1 is on monitor 1 and workspace 5 is on monitor 2, it would display only the focused workspace, which is either 1 or 5 depending on which monitor I'm on.

@Diaoul
Copy link

Diaoul commented Oct 9, 2023

I'm looking for a way to have active workspace (e.g. 9) on a not focused monitor (e.g. B) to have a different style applied to it on the bar on monitor B compared to other workspaces there.

Reading this, I'm not sure it is possible nor which combination to try 😕

Edit: Running git version seems to have improved things!

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.

6 participants