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

foreign-toplevel-manager based taskbar module #692

Merged
merged 12 commits into from
May 30, 2020

Conversation

l3nkz
Copy link
Contributor

@l3nkz l3nkz commented May 4, 2020

Although waybar was originally designed to work with sway, some people use it as a bar with other compositors now. People such as me ;) One feature I was missing from waybar, was a simple taskbar with all the windows that I have open (similar to tint2 for X). After some searching I found wf-panel and its taskbar implementation based on the foreign-toplevel-manager protocol defined by wlroots. I took a lot of inspiration from this implementation and developed a module with similar features for waybar.

@Alexays
Copy link
Owner

Alexays commented May 4, 2020

Oh nice stuff, i'll review it when i'll get some time 😃

@Alexays
Copy link
Owner

Alexays commented May 19, 2020

You forgot to add man file to build https://github.com/Alexays/Waybar/blob/master/meson.build#L227
Code LGTM apart the code style, but i'll take care of it in an upcoming PR.

Nice work!
Great addition so far!

When you added the man file to build, can you update the github wiki and i'll merge this PR ❤️

@l3nkz
Copy link
Contributor Author

l3nkz commented May 20, 2020

I pushed the change that adds the man page generation for the taskbar.

The PR now has conflicts with master. Do you want me to rebase my branch on the current master and force push? Or do you want to resolve the conflicts yourself.

Furthermore, I had some crashes recently and I am still trying to figure out what happens there. The crash occurs when I unplug my external monitor. First the bar gets unresponsive (doesn't update texts, …) and after some time crashes completely with the following message:

waybar: ../src/ierror.h:130: mpd_error_get_message: Assertion `error->message != NULL || error->code == MPD_ERROR_OOM' failed.

I still have to look in the core dump.

@Alexays
Copy link
Owner

Alexays commented May 20, 2020

Conflicts fixed :)
For the crash, It's probably from a module destrutor, obviously the mpd's destrutor here.

@l3nkz
Copy link
Contributor Author

l3nkz commented May 20, 2020

Ok, so it is not related to my changes … I will double check with the original master branch without my module when I have time.

@Alexays Alexays merged commit adaf843 into Alexays:master May 30, 2020
@Alexays
Copy link
Owner

Alexays commented May 30, 2020

Just merged!
Can you update the github wiki?
Your issue should be fixed with latest master changes

@Alexays
Copy link
Owner

Alexays commented May 30, 2020

Thanks!

This was referenced May 30, 2020
@l3nkz
Copy link
Contributor Author

l3nkz commented Jun 24, 2020

Just merged!
Can you update the github wiki?
Your issue should be fixed with latest master changes

I updated the wiki now. Sorry for the long delay. I was very busy the last weeks.

@Anakael
Copy link
Contributor

Anakael commented Jun 24, 2020

Hello!
I try to launch taskbar and get error:
[error] Failed to register as toplevel manager
In config i have:
"modules-center": ["wlr/taskbar"],
Maybe I need latest sway version? (Now I have the one from community arch repo).
Great work! Interesting, how you solve problem with icon. Need to study your solution :)

@l3nkz
Copy link
Contributor Author

l3nkz commented Jun 24, 2020

Hm, it seems as if your compositor doesn't support the foreign-toplevel-manager protocol. This is a limitation of sway (See issue #728). But according to the issue using the git version of sway from the AUR might solve the problem.

Which problem with the icon do you mean?

@Anakael
Copy link
Contributor

Anakael commented Jun 24, 2020

Problem that wayland doesn't have xprop analog and icon path for application is not fixating.

@Anakael
Copy link
Contributor

Anakael commented Jun 24, 2020

I just rebuild sway with wlroots from master branches and there is still the same issue.
With which compositor did you test it?

@Anakael
Copy link
Contributor

Anakael commented Jun 24, 2020

It works now!
But is there way for separate task bar for tag?
And there is no icon on tasks...

@Anakael
Copy link
Contributor

Anakael commented Jun 24, 2020

And close on middle-click doesn't work.

@Anakael
Copy link
Contributor

Anakael commented Jun 25, 2020

About middle-click:
Found out that is mistake in example. Work fine. I will commit that fix if manage to solve another tasks in taskbar.

@Anakael
Copy link
Contributor

Anakael commented Jun 25, 2020

-> And there is no icon on tasks...
Oh, that bug still not fixed.
You set icon in app_id subscription. But not all nodes in sway have that property.
For example: Alacritty has, but firefox, visual-studio-code, gnome-terminal - hasn't.
I will try to search sway about that.

@l3nkz
Copy link
Contributor Author

l3nkz commented Jun 25, 2020

About middle-click:
Found out that is mistake in example. Work fine. I will commit that fix if manage to solve another tasks in taskbar.

Good catch. I fixed the wiki page. Thank you.

I just rebuild sway with wlroots from master branches and there is still the same issue.
With which compositor did you test it?

I use wayfire.

-> And there is no icon on tasks...
Oh, that bug still not fixed.
You set icon in app_id subscription. But not all nodes in sway have that property.
For example: Alacritty has, but firefox, visual-studio-code, gnome-terminal - hasn't.
I will try to search sway about that.

Hm, that is interesting. I will also have a look into this. But this might be a bug of sway. Because if I don't get an app_id, I don't really know how to identify the app. The title is usually not very helpful.

@Anakael
Copy link
Contributor

Anakael commented Jun 25, 2020

It's not really bug, but nuance of Wayland and sway. app_id used only for pure Wayland application and missing in xwayland.
For xwayland apps .class property can be useful.

@l3nkz
Copy link
Contributor Author

l3nkz commented Jun 25, 2020

Unfortunately, .class is not provided to the module via the foreign-toplevel-manager protocol. Hence I would say this is a problem of sway that it doesn't translate the .class property into the app_id. wayfire which is based on wlroots apparently does it this way. Because there I don't have any problem with missing icons also for xwayland apps.

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.

None yet

3 participants