Skip to content

Expo: add keyboard interaction #1156

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

Merged
merged 7 commits into from
May 27, 2021
Merged

Conversation

dkondor
Copy link
Contributor

@dkondor dkondor commented Apr 23, 2021

Fixes #1142

Arrows keys can be used to select a target workspace, enter to change, esc to cancel.

Also added shading to unselected workspaces.

TODO:

  • Make this optional (add a bool option)
  • Option for the shading of unselected workspaces instead of fixed value of 70%
  • Animation when the selected workspace changes
  • Handle key repeat (when a key is held down)
  • Make keys configurable? Postponed for later
  • Decide what should happen if multiple keys are pressed / keys are pressed while dragging a view? Only the last pressed key is used, keys are disabled while dragging a view.
  • Test for regressions in other places where workspace-wall is used
  • Test the case when the grid size changes while expo is running

Copy link
Member

@ammen99 ammen99 left a comment

Choose a reason for hiding this comment

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

I know this isn't ready but had some time for reviews today so here you go :)

@@ -71,8 +74,18 @@ class wayfire_expo : public wf::plugin_interface_t
} state;

int target_vx, target_vy;
int initial_vx, initial_vy;
Copy link
Member

Choose a reason for hiding this comment

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

use wf::point_t in new code :) makes it easier to use if we need to pass the points around.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, I changed for both (didn't make sense to have separate x/y for target if initial workspace is wf::point_t)

std::unique_ptr<wf::workspace_wall_t> wall;

wf::option_wrapper_t<int> delay{"input/kb_repeat_delay"};
Copy link
Member

Choose a reason for hiding this comment

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

Feels like we're duplicating code already written here: https://github.com/WayfireWM/wayfire/blob/master/plugins/scale/scale-title-filter.cpp#L19

Maybe we can extract that struct in plugins/common/... and re-use it whenever we need key repeat?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that makes sense; I've moved it there

@ammen99
Copy link
Member

ammen99 commented May 21, 2021

[ ] Make keys configurable?

If you're not particularly interested in this, I think it should be fine to just handle HJKL and leave this for later.

@dkondor dkondor marked this pull request as ready for review May 22, 2021 16:32
@dkondor
Copy link
Contributor Author

dkondor commented May 22, 2021

I've addressed the code review comments and did some more testing, so I think this is now ready :)

Copy link
Member

@ammen99 ammen99 left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@ammen99 ammen99 merged commit 1158284 into WayfireWM:master May 27, 2021
@dkondor dkondor deleted the expo_keyboard branch June 5, 2021 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Escape from expo plugin
2 participants