Synchronize decorations buttons order in GTK headerbars
ClosedPublic

Authored by gikari on Dec 1 2019, 11:12 PM.

Details

Summary

Window decorations button order was applied only for window headers that was controlled by KWin, but not for GTK applications with CSD. Now it is no longer true - button order in CSD applications are in sync with the one used by KWin.

Only Close, Maximize, Minimize and Icon buttons are synchronized, because GTK supports only them.

Depends on D25695

Test Plan
  1. Open two windows alongside each other: window decorations button order settings and any gtk3 app with CSD (for example, Lutris)
  2. Restart kded5
  3. Apply any WD button order, apply settings
  4. The app should change its buttons order in headerbar (if xsettingsd is not installed, on X11 only after restart)

Diff Detail

Repository
R99 KDE Gtk Configuration Tool
Branch
gtk-decoration-order-sync (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 19406
Build 19424: arc lint + arc unit
gikari created this revision.Dec 1 2019, 11:12 PM
Restricted Application added a project: Plasma. · View Herald TranscriptDec 1 2019, 11:12 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
gikari requested review of this revision.Dec 1 2019, 11:12 PM
gikari edited the test plan for this revision. (Show Details)Dec 1 2019, 11:22 PM
cblack added a comment.EditedDec 1 2019, 11:27 PM

You would probably want to use icon instead of menu, as menu is only rendered as a fallback for a pattern that most GNOME applications don't use nowadays.

Edit: Additionally, GTK apps that use an appmenu typically override the window decoration layout to reflect this. If they don't, then that's an issue on their end.

gikari added a comment.EditedDec 2 2019, 4:26 AM

You would probably want to use icon instead of menu, as menu is only rendered as a fallback for a pattern that most GNOME applications don't use nowadays.

Edit: Additionally, GTK apps that use an appmenu typically override the window decoration layout to reflect this. If they don't, then that's an issue on their end.

I would like to say "It is their issue", but the thing is, if we remove appmenu from that apps, they will be unusable, because this menu contains important functionality, that is not present in the other parts of an app. If I choose to just grab icon instead of menu, apps would break and the user will be confused and unable to use these apps.

Edit: As I discovered, the BleachBit appmenu and Hambuerger menu duplicate each other, so for that app it would be safe to remove appmenu. However for Lutris it not true. They have an issue on their issue tracker about this.

broulik added a subscriber: broulik.Dec 2 2019, 8:03 AM

It would be sensible to use an individual signal. I would like to hear an advice about its naming.

Alternatively you could have the KWin decoration KCM use writeEntry with Notify flag and then use a KConfigWatcher here instead of connecting to KWin's generic DBus signal.

gikari updated this revision to Diff 70778.Dec 2 2019, 9:06 PM
gikari edited the test plan for this revision. (Show Details)
  • Use KConfigWatcher instead of KWin dbus signal
gikari updated this revision to Diff 70782.Dec 2 2019, 9:28 PM
  • Change appmenu to icon
gikari edited the summary of this revision. (Show Details)Dec 2 2019, 9:29 PM
GB_2 added a subscriber: GB_2.Dec 14 2019, 4:30 PM

Works great!

apol added a subscriber: apol.Dec 18 2019, 5:04 PM

This looks like it's trying to change configurations from other sources. This feels wrong.

In D25670#579888, @apol wrote:

This looks like it's trying to change configurations from other sources. This feels wrong.

Could you elaborate on that? What do you mean exactly? What other sources?
Do you mean that org.gnome.desktop.wm.preferences path is somewhat wrong, because it is different from standard org.gnome.desktop.interface?

apol accepted this revision.Dec 18 2019, 5:36 PM

Never mind my last message, that's okay.

This revision is now accepted and ready to land.Dec 18 2019, 5:36 PM
gikari updated this revision to Diff 71810.Dec 18 2019, 5:51 PM

Rebase on master

This revision was automatically updated to reflect the committed changes.
trmdi added a subscriber: trmdi.Sep 5 2020, 2:24 AM
This comment was removed by trmdi.