search

wmww / gtk-layer-shell

A library to create panels and other desktop components for Wayland using the Layer Shell protocol
GNU General Public License v3.0
318 stars 16 forks source link

New keyboard interactivity options #105

Closed dkondor closed 3 years ago

dkondor commented 3 years ago

Support more settings for keyboard interactivity, according to the changes suggested to the protocol here: https://github.com/swaywm/wlr-protocols/pull/100

By opening this pull request, I agree for my modifications to be licensed under whatever licenses are indicated at the start of the files I modified

emersion commented 3 years ago

@wmww Ack on merging the wlr-protocols PR?

wmww commented 3 years ago

Now that the upstream PR has merged I'll give this a full review when I have time. These don't block my initial review, but they will need to be done before merging:

P.S. let me know if anything about writing tests could be documented better.

dkondor commented 3 years ago

Now that the upstream PR has merged I'll give this a full review when I have time. These don't block my initial review, but they will need to be done before merging:

* Pull in the final version of the layer shell protocol

Done already :)

* Remove the new example (we don't need individual examples for every feature, adding interactivity support to the demo is enough)

I'm not sure what you mean; the new files are part of the demo, just separated for clarity (I can merge them with one of the other ones).

* Add integration tests (see [test-set-keyboard-interactivity.c](https://github.com/wmww/gtk-layer-shell/blob/master/test/integration-tests/test-set-keyboard-interactivity.c), [test-get-keyboard-interactivity.c](https://github.com/wmww/gtk-layer-shell/blob/master/test/integration-tests/test-get-keyboard-interactivity.c) for examples, and [test/README.md](https://github.com/wmww/gtk-layer-shell/blob/master/test/README.md) for full test documentation (though you shouldn't need to understand most of it))

I did a first try at this by adding the following two files: test/integration-tests/test-get-keyboard-interactivity-types.c test/integration-tests/test-set-keyboard-interactivity-types.c

To make this work, I had to update the mock server to advertise version 4 of the protocol. I'm wondering if there should be an extra test that gtk-layer-shell behaves as expected on a lower server version and if that's easily achievable with the current setup.

dkondor commented 3 years ago

Also, I'm marking this ready for review since the protocol and wlroots PRs have merged and the main functionality is implemented.

wmww commented 3 years ago

I'm not sure what you mean...

Either the thing I was talking about was removed or I was just confused. Either way it's fine now.

wmww commented 3 years ago

All right as you can see I've added a bunch of changes, including notably changing the names from "keyboard_interactivity_type" to "keyboard_mode". That diverges from the protocol terminology, but I think it's a little clearer, much shorter and less likely to be confused with the deprecated functions. Much appreciated if you want to give the PR a review in it's current state. It seems ready to merge to me.

dkondor commented 3 years ago

All seems good, I've only added a minor edit in one place where the documentation was inconsistent.

Otherwise, I've just tested a bit and seems to work well.

wmww commented 3 years ago

Awesome! Thanks!