can-lehmann / owlkettle

A declarative user interface framework based on GTK 4
https://can-lehmann.github.io/owlkettle/README
MIT License
375 stars 13 forks source link

Add support for window controls (aka decorationLayout) to headerbar #123

Closed PhilippMDoerner closed 11 months ago

can-lehmann commented 11 months ago

From what I can tell, one of the major applications of WindowControls is creating split header bars (like we had in the Slider example at some point). The problem they solve here is allowing the user to select on which side of the window the close button should be shown. This is also why the side field exists.

For this application we need to provide the option to not override GtkSettings:gtk-decoration-layout.

PhilippMDoerner commented 11 months ago

Could you expand what conclusion you want me to draw from this ? Mostly because I can draw multiple.

E.g. the simplest interpretation would be to say "if seq[WindowControlButton].len == 0 then either don't set decoration-layout string or set it to the default-value" (both achieve the same result of having the default value present).

I'm just not sure if you mean that or want me to go in the direction of splitting "WindowControlButton" into "Left" and "right" or want me to abandon the approach with a "WindowControlButton-enum" altogether.

can-lehmann commented 11 months ago

Could you expand what conclusion you want me to draw from this ? Mostly because I can draw multiple.

The main point is allowing the user to explicitly leave/make unset. Note that this unset state is explicity different from adding no buttons. One way to do this is using buttons: Option[seq[WindowControlButton]] (or as below buttons: Option[DecorationLayout]). It needs to explicitly read gtk-decoration-layout if left unset.

Other related changes that might be interesting:

  1. Create a "DecorationLayout" type that has fields for left/right
  2. Make buttons a setter is overloaded to support both DecorationLayout (and immediately calls $ on it) and a regular string
PhilippMDoerner commented 11 months ago

I have just now learned that Headerbar also has a decorationLayout string. Doing effectively the exact same.

Could it make sense to abandon wrapping WindowControls for the same reason that wrapping Bin was abandoned (it's kinda pointless) and instead add wrapping for decorationLayout to HeaderBar?

Because the way I currently see it nobody should be using the WindowControls widget if they can just use HeaderBar. Providing a wrapping with e.g. a DecorationLayout tuple type and a setter is something I'm happy to do there because the API is far less confusing in how HeaderBar' uses it to render both buttons on the left and right. And also it has no confusingside` property to deal with.

WindowControls seems more like an internal Widget than one an end-user should actually use. As such I'm tempted (even though I did just finalize your suggestions), to delete WindowControls entirely out of owlkettle so that you only set them via headerbars.

PhilippMDoerner commented 11 months ago

I've updated the PR to reflect what we just discussed in discord:

This PR is no longer for adding the windowcontrols widget. It is for adding support of those window controls (including a nicer setter) to Headerbar. I removed the example since headerbar previously didn't have one so I assume it was deemed too small to be worth an example widget.

can-lehmann commented 11 months ago

I just saw that Gtk.HeaderBar:decoration-layout also overrides GtkSettings:gtk-decoration-layout. So the field should by of type Option[string] to allow for explicitly not overriding the property.

PhilippMDoerner commented 11 months ago

@can-lehmann I'm closing this PR and merging it into #116 . That PR with AdwHeaderBar also needs decorationlayout and basically all the same hooks, enums and toLayoutString procs etc.

It makes sense to develop double / have to refactor that after merge and instead combine them right now. As per your previous statement that you prefer "coupled" PRs to be combined, I am thus combining them.