Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for layer-shell on_demand keyboard interactivity #7587

Merged
merged 1 commit into from
Jun 6, 2023

Conversation

ErikReider
Copy link
Contributor

@ErikReider ErikReider commented May 13, 2023

Supersedes #5974

Tested (from original PR) with gtk-layer-demo:

  • xdg toplevel + layer-surface/top/on-demand
  • xdg toplevel + layer-surface/top/on-demand + layer-surface/top/exclusive
  • xdg toplevel + layer-surface/top/on-demand + layer-surface/overlay/exclusive
  • layer-surface/top/exclusive + layer-surface/top/none: Should be able to interact with none layer, popups, and subsurfaces(buttons, no keyboard input)
  • focus_follows_mouse
  • layer-surface/top/on-demand + layer-surface/top/exclusive, on-demand only focused when focused inside of bounds while exclusive is focused otherwise (also outside of its bounds)
  • layer-surface/top/exclusive + layer-surface/top/exclusive: Layer has focus until the cursor enters/clicks within the other layers bounds
  • Keyboard input is only available for EXCLUSIVE and ON_DEMAND surfaces
  • Interact buttons in layers/subsurfaces/popups while NONE exclusive
  • Clicking on xdg_popups/layer sub-surfaces works with any mode
  • return keyboard focus after destroying on-demand layer surface

More test cases would be appreciated :)

@kennylevinsen
Copy link
Member

This PR allows for entering text into textboxes while the layer is focused.

How was the bug reproduced? Is it with or without keyboard exclusivity?

The git history of this PR is a little foo. Having a commit for each of the fixes with a title and description that fits would be best.

@ErikReider
Copy link
Contributor Author

How was the bug reproduced? Is it with or without keyboard exclusivity?

This was tested with a layer without any keyboard exclusivity (a notification from swaync with this PR). In the video below, the popup notification was tested while the second layer has keyboard exclusivity and covers the whole output (it's transparent).

2023-05-16.11-09-53.1.mp4

Prior to the fixes, you'd be able to focus and move views below an exclusive layer shown in the video below:

2023-05-16.11-19-57.1.mp4

The git history of this PR is a little foo. Having a commit for each of the fixes with a title and description that fits would be best.

I'm on it!

@ErikReider ErikReider force-pushed the layer-shell-focus-fixes branch from 273d486 to 37d1390 Compare May 16, 2023 10:47
@ErikReider
Copy link
Contributor Author

I removed the commit that disallowed non-keyboard exclusive layers from receiving mouse clicks while there being a exclusive layer beside it (ex: waybar receiving clicks while rofi is visible), so ignore that part in the videos above.

@kennylevinsen
Copy link
Member

Hmm I'm a little confused between the original PR description and what the video displays.

A few notes about keyboard interactivity:

  • It is only meant to affect (and if exclusive, steal) keyboard input and focus. Pointer, touch and tablet input and focus is not constrained and should always work as normal. This seems to be changed here.
  • Keyboard interactivity defaults to none, where keyboard focus should never be granted to the surface.
  • For exclusive, given a surface in TOP or OVERLAY, the top-most surface should be granted unconditional keyboard focus as long as it is mapped.
  • For on_demand, the layer surface should behave like a normal surface.

More info here: https://wayland.app/protocols/wlr-layer-shell-unstable-v1#zwlr_layer_surface_v1:enum:keyboard_interactivity

SwayNotificationCenter popups with a text field would definitely want on_demand. sway does not seem to currently handle on_demand, and might be treating it as exclusive by accident as it just checks if (keyboard_interactivity), only distinguishing between 0 (none) and anything else. We need to make the handling of the two original types explicit checks, allowing us to fall back to normal focus logic for on_demand.

@ErikReider
Copy link
Contributor Author

ErikReider commented May 21, 2023

Makes sense. So adding a check if the surface is requesting on_demand would be fine then?

@ErikReider
Copy link
Contributor Author

ErikReider commented May 21, 2023

I've got the changes necessary to advertise ON_DEMAND, should I create a new PR and close this one or change the title, desc, and redo the commits? @kennylevinsen

@kennylevinsen
Copy link
Member

Either works!

@ErikReider ErikReider changed the title Allow layershell text input while focused Add support for layer-shell on_demand keyboard interactivity May 21, 2023
@ErikReider
Copy link
Contributor Author

Updated everything, would you like a clean commit history too?

sway/input/seatop_default.c Outdated Show resolved Hide resolved
sway/input/seatop_default.c Outdated Show resolved Hide resolved
sway/desktop/layer_shell.c Outdated Show resolved Hide resolved
@ErikReider ErikReider force-pushed the layer-shell-focus-fixes branch from a522cd7 to 08d4626 Compare May 22, 2023 14:42
Copy link
Member

@kennylevinsen kennylevinsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking better! Still a thing remaining though. :)

sway/input/seatop_default.c Outdated Show resolved Hide resolved
@ErikReider ErikReider force-pushed the layer-shell-focus-fixes branch from 21ed68b to 7533e3f Compare May 24, 2023 14:48
@ErikReider
Copy link
Contributor Author

Anything else that needs to be done?

@ErikReider ErikReider force-pushed the layer-shell-focus-fixes branch 2 times, most recently from d7ed064 to 12274f2 Compare June 5, 2023 07:25
Copy link
Member

@kennylevinsen kennylevinsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@kennylevinsen
Copy link
Member

@ErikReider Do you want to reword the git commits before we merge? They talk about the original version which had the wrong focus behavior. A conflict also snuck in in the meantime.

Will be merged when that is out of the way. :)

This allows for layer shell surfaces to receive focus while the surface is explicitly focused, i.e allowing
text fields to receive keyboard input just like a regular surface.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants