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

Improve keyboard navigation #463

Merged
merged 6 commits into from
Oct 1, 2023
Merged

Improve keyboard navigation #463

merged 6 commits into from
Oct 1, 2023

Conversation

oscfdezdz
Copy link
Collaborator

No description provided.

@oscfdezdz
Copy link
Collaborator Author

oscfdezdz commented Aug 19, 2023

  • When using the app with keyboard only I came across a few situations where the app loses focus, and I was not able to get it back without using the mouse:

  • Flipping a toggle to turn on an extension

  • Opening an image preview

I tried to track down these issues of GNOME Circle application to include a fix in this PR, without much success.

  • Flipping a toggle to turn on an extension

I couldn't figure out why this one happens. When toggling a switch from one of the groups, user-installed or system, the first switch of the group always grabs the focus.

  • Opening an image preview

This button is focusable, although when pressed, focus can go behind the overlay using the keyboard which is probably undesirable. The solution here could be to intercept key press events and return the focus if the overlay is active and moves behind it, but maybe I'm just overthinking it.

@mjakeman
Copy link
Owner

mjakeman commented Oct 1, 2023

I've opened #485 to track the remaining issues. I think this is a good start.

Regarding the extension toggle, of the top of my head this is because it recreates the entire group rather than updating the individual row state (it might not actually be the case, I wrote that code nearly two years ago 😅).

We should get this for free with the unified extensions model from #67 which is prototyped in gnome-circle.

@oscfdezdz Could you rebase this please? Then happy to merge

@oscfdezdz
Copy link
Collaborator Author

@oscfdezdz Could you rebase this please? Then happy to merge

It's ready now, I also added the action to close the comments dialog with Esc.

@mjakeman mjakeman merged commit 9163828 into mjakeman:master Oct 1, 2023
1 check passed
@oscfdezdz oscfdezdz deleted the shortcuts branch October 1, 2023 09:43
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.

None yet

2 participants