Closed masozzi closed 5 years ago
This looks cool, but it might be worth using gtk-layer-shell to make it easier to maintain
I totaly forgot about that... i'll give it a try. Thanks!
@masozzi The two things you outlined, plus the fact that you can't interactively drag a window with layer-shell, is the reason why I use wf-shell. Do you think we could have both backends? That way, the keyboard will work on sway, and it will continue working with the full feature set in wayfire.
Do you think we could have both backends?
yes, i think we could have both. although, i think managing the keyboard options for both wf-shell and layer-shell will become a little messy; we would end up with some options that would work just for wf-shell and other just for layer-shell
@masozzi What options would work for just wf-shell and just for layer-shell? Anyway, i would be fine with it.
For example: x and y coordinate are just for wf-shell. For layer-shell, we need to add the option to change the default anchor. Optionally we can add the margin.
wf-shell also supports positioning via anchors and margins.
@ammen99 Sorry if took long.
so, i've added clara which is a hendy helper for parsing command line options; and the support for anchors. also, wf-shell is back alogn side with layer-shell.
let me know
@masozzi, I'll do an in-depth review later. At first glance though, do we need clara when we have getopt_long
? What does it let us achieve we can't otherwise?
@ammen99
do we need clara when we have getopt_long?
honestly is just to have a cleaner code, and easier to add new options (if needed). I can got for getopt_long if you prefer, let me know.
@ammen99 Fixed all style issue.
Do you think it would make sense to use https://github.com/wmww/gtk-layer-shell ? It would remove a lot of the needed boilerplate.
yes, but keep in mind that https://github.com/wmww/gtk-layer-shell is a library, so it would require one more dependency, but that's not a big deal. Would you prefer a separate pull-request for this, or should i add it to this?
Also, there is still no way to close the keyboard on sway: do you think that adding a key to quit can be done? you have other ideas?
I think we can just add gtk-layer-shell as a submodule if it isn't present on the system. Meson has support for this via the fallback parameter for dependencies. I would prefer to have this happen in this PR.
And yes, adding a key to close the keyboard can be done pretty easily by adding a new special keycode (if you look at that ugly hack which represents buttons)
However, IIRC we can add a GtkHeaderbar as a widget - in which case it will provide a close button. We need this only in the layer-shell case.
Adding a header bar for a close button is a bit overkill I think
@AdrianVovk, what to do otherwise?
@ammen99 Just have a button and on click do window.destroy(). Adding in an entire headerbar just to close the osk is a bit much
After some consideration I think the way forward is using only layer-shell with potentially optional behavior with wayfire-shell (wayfire shell will be redesigned to be used together with layer-shell). So I think the best would be a headerbar with close button, and this headerbar would also start an interactive drag in the future.
@ammen99
After some consideration I think the way forward is using only layer-shell with potentially optional behavior with wayfire-shell (wayfire shell will be redesigned to be used together with layer-shell)
This means that layer-shell should have the priority over wf-shell? or the priority should remain to wf-shell, and in case it does not exist we go for layer-shell?
layer-shell should have all priority. I am sorry for wasting your time in requesting that you support both. You can see what wayfire-shell will be like in the future: https://github.com/WayfireWM/wf-shell/blob/layer-shell/proto/wayfire-shell-unstable-v2.xml
So it will be all layer-shell and wayfire-shell will only provide support for the interactive drag (which I may implement later).
About gtk-layer-shell, you can add it as a subproject as I did in the layer-shell branch of wf-shell.
@ammen99 i saw that you have include the commits from this PR into the layer-shell-gtk
branch (plus some modifications), should i close this?
@masozzi I was hoping you could test my changes, hopefully I didn't break anything. Then you can merge the PR and I'll merge this one (which will then include also my commits)
@ammen99 i have tested your branch, happy to report that works.
@masozzi Thanks for working on this initially and for testing :)
I have changed the protocol from wf-shell to layer-shell. This has only been test on sway, i still need to test i on wayfire.
problems found so far:
Apart for those two anoying things, everything else seems to work.