Open jpnurmi opened 1 year ago
Eventually we might need to increase the width of the very small compact sidebar by some pixels for this then
Some issues highlighted by Tweaks.
With separated title bars, there's often no room in the leading area. Tweaks has to increase the width of the master pane...
Screencast from 2023-01-16 09-21-26.webm
In portrait mode, the button arrangement gets a bit cumbersome. Notice also a rendering issue in the rounded top window corners. | Master | Detail |
---|---|---|
Just submitted #762 , which would allow specifying a custom window control layout.
This way, a developer can choose to respect the platform's current window control layout — by using the YaruWindowControlLayout.parseGTKSetting(settingString)
in combination with the gtk
package, which can retrieve this GTK setting string.
There are several potentially controversial design choices I made that warrant additional discussion:
gtk
package. I decided to not add an extra dependency and keep the library lightweight, but it's worth considering whether Yaru shouldn't just always respect the appropriate GTK setting, without extra work by the developer.isCloseable
, isMaximizable
, isRestorable
, and isMinimizable
. It seems to me that, with the new YaruWindowControlLayout
and nullable onClose
/onMaximize
/onRestore
/onMinimize
callbacks, these attributes are no longer needed. These would also need to be removed from yaru_window_platform_interface
, though.YaruWindowControlType
for specifying layouts and treating "maximize" and "restore" as the same button. It didn't make sense to me to allow a different position for the "restore" button than for the "maximize" button, or to always specify both in the layout. That's why I treat this button type the same way and it doesn't matter if a layout specifies "maximize" or "restore" as long as it doesn't specify both. It might be worth creating a different enum for this altogether, with maximize/restore as a single entry.What do you think about these points?
Let me know what the best way forward is.
Hi @12people Thank you very much :+1: Generally not pulling more deps in is very wise. I can not say much about the rest as what you say sounds plausible though needs maybe a bit more testing. Or is this something that could be instead made in YaruWindow? (a dep of yaru widgets)
Even though this could take time or will happen never, I would prefer to see an opinion of @jpnurmi on this one.
@12people we def. want this in, thank you very much for your work. But since I did not write those parts but JP (which sadly left) we need to be sure that it works. I hope I will find soon time to properly review this
No problem.
I should just mention that before this is merged, the issues brought up above need to be resolved and also tests need to be adjusted (at least goldens — there will be new ones needed).
@Feichtmeier What state is this in? The last comment here mentions that the PR needs to be reviewed — has that happened? What are the next steps?
@Feichtmeier What state is this in? The last comment here mentions that the PR needs to be reviewed — has that happened? What are the next steps?
This is the current status It has conflicts now, as I can see you changed the flutter and the part that watches the gtk settings is still needed,right?
@Feichtmeier The part that loads gtk settings can be left up to the gtk
package, as mentioned above. My code doesn't include this library in order to not add an extra dependency, but, if you decide, it can be included.
So the pull request is still waiting for review, if I understand correctly?
So the pull request is still waiting for review, if I understand correctly?
This repo here now depends on gtk anyways because YaruTheme also watches for a gtk setting (the gnome theme) So including this into your PR would be a good idea https://github.com/ubuntu/yaru.dart/blob/main/pubspec.yaml#L17 After you added it I will review it
@Feichtmeier You assigned this to me, but I don't have enough dart knowledge for this...
Okay sorry, I thought because you created the PR? :) Anyways, sorry then.
Oh, ok... yeah, now I see it. I tried, but certainly it is quite out of my current capabilities :-D
We need to ensure that this works with multiple headerbar scenarios
titlebar actions (lower, menu, minimize, none, toggle maximize, toggle maximize horizontally, toggle maximize vertically, toggle shade)
titlebar buttons
see also