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

Port TabSwitcher, Show Desktop & Workspace applets over to libxfce4windowing #485

Merged
merged 11 commits into from
Oct 29, 2023

Conversation

JoshStrobl
Copy link
Member

@JoshStrobl JoshStrobl commented Oct 22, 2023

This PR ports the following features over to libxfce4windowing from using libwnck directly:

  • Show Desktop applet
  • Workspaces Applet
  • TabSwitcher

Info

TabSwitcher

To facilitate tab switcher port and cleanup, considerable amounts of the code was gutted or rewritten, for example wm no longer does any sort of cur_tabs tracking. That is left up to TabSwitcher.

Instead of rebuilding tabswitcher contents every single time we invoke it, we allow it to handle its own state and handle changes based on libxfce4windowing events. Windows are added / removed automatically, workspace handling is done to support show-all-windows function and workspace swapping, and we leverage sort with our recency list and filter to know when to hide items.

Show Desktop applet

This code implements feature parity with Show Desktop without separately recording any window states. Instead, we just increment over all our stacked windows on the screen and set the minimization state for each window based off the widget toggled state, which gets updated on the screen window_opened and window state changed events.

Workspaces Applet

Nothing really special to note here about the port. I will however highlight a known issue with Workspace Applet port, which is that windows may not report correct icon. While this is corrected in TabSwitcher with use of AppSystem, I'm going to refrain from duplicating this code into the Workspace applet as I have to then address some issues with AppSystem instantiation triggering segfaults that may otherwise not exist once we centralize it with @EbonJaeger's Windowing namespace / class in his IconTasklist branch, and don't think it's worth adding in more code that'll need to be cleaned up for what is effectively an issue end users won't experience in final release all consumers of similar logic will have been cleaned up by then.


Testing Notes

Be sure to reboot to ensure you are using correct budgie-wm, budgie-daemon, budgie-panel.

This PR ports our TabSwitcher over to libxfce4windowing from using libwnck directly. To facilitate this, considerable amounts of the code was gutted or rewritten, for example wm no longer does any sort of cur_tabs tracking. That is left up to TabSwitcher.

Instead of rebuilding tabswitcher contents every single time we invoke it, we allow it to handle its own state and handle changes based on libxfce4windowing events. Windows are added / removed automatically, workspace handling is done to support show-all-windows function and workspace swapping, and we leverage sort with our recency list and filter to know when to hide items.
@JoshStrobl JoshStrobl added the wayland Tasks related to port to Wayland label Oct 22, 2023
@JoshStrobl JoshStrobl added this to the 10.9 milestone Oct 22, 2023
@JoshStrobl JoshStrobl requested a review from a team October 22, 2023 15:29
@JoshStrobl JoshStrobl self-assigned this Oct 22, 2023
@JoshStrobl JoshStrobl linked an issue Oct 22, 2023 that may be closed by this pull request
@JoshStrobl JoshStrobl linked an issue Oct 22, 2023 that may be closed by this pull request
@JoshStrobl JoshStrobl changed the title Port TabSwitcher over to libxfce4windowing Port TabSwitcher and Show Desktop applet over to libxfce4windowing Oct 22, 2023
Copy link
Member

@EbonJaeger EbonJaeger left a comment

Choose a reason for hiding this comment

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

Looks pretty good for a first look over. One thing I notice is that the tab switcher children don't directly subclass FlowBoxChild. I wonder if it would be a good idea to do that; it would avoid having to get_parent() and get_child() whenever you want to do something to them.

src/daemon/tabswitcher.vala Show resolved Hide resolved
src/daemon/tabswitcher.vala Outdated Show resolved Hide resolved
src/daemon/tabswitcher.vala Outdated Show resolved Hide resolved
src/daemon/tabswitcher.vala Outdated Show resolved Hide resolved
src/daemon/tabswitcher.vala Outdated Show resolved Hide resolved
@JoshStrobl JoshStrobl linked an issue Oct 27, 2023 that may be closed by this pull request
@JoshStrobl JoshStrobl changed the title Port TabSwitcher and Show Desktop applet over to libxfce4windowing Port TabSwitcher, Show Desktop & Workspace applets over to libxfce4windowing Oct 27, 2023
@JoshStrobl
Copy link
Member Author

@EbonJaeger I addressed your feedback on the idea of sub-classing GtkFlowBoxChild in b08f5fe 👍 Ready for another round of feedback! 😁

@JoshStrobl
Copy link
Member Author

Known issue with Workspace Applet port: Windows may not report correct icon. While this is corrected in TabSwitcher with use of AppSysstem, I'm going to refrain from duplicating this code into the Workspace applet as I have to then address some issues with AppSystem instantiation triggering segfaults that may otherwise not exist once we centralize it with @EbonJaeger's Windowing namespace / class in his IconTasklist branch, and don't think it's worth adding in more code that'll need to be cleaned up for what is effectively an issue end users won't experience in final release all consumers of similar logic will have been cleaned up by then.

Going to add this to OP as well.

Copy link
Member

@EbonJaeger EbonJaeger left a comment

Choose a reason for hiding this comment

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

Awesome stuff! Even though it is a bit of a pain having multiple ports in one PR, it would also be a pain to have them separate 😛

src/daemon/tabswitcher.vala Outdated Show resolved Hide resolved
src/daemon/tabswitcher.vala Outdated Show resolved Hide resolved
src/panel/applets/workspaces/WorkspacesApplet.vala Outdated Show resolved Hide resolved
JoshStrobl and others added 3 commits October 28, 2023 21:27
Co-authored-by: Evan Maddock <[email protected]>
Co-authored-by: Evan Maddock <[email protected]>
Co-authored-by: Evan Maddock <[email protected]>
Copy link
Member

@EbonJaeger EbonJaeger left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! :)

@JoshStrobl JoshStrobl merged commit 74b6564 into main Oct 29, 2023
1 check passed
@JoshStrobl JoshStrobl deleted the 474-implement-wayland-support-in-tabswitcher branch October 29, 2023 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wayland Tasks related to port to Wayland
Projects
Status: Done
2 participants