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

Keyboard focus fixes #211

Merged
merged 14 commits into from
Dec 31, 2023
Merged

Keyboard focus fixes #211

merged 14 commits into from
Dec 31, 2023

Conversation

white-axe
Copy link
Contributor

This pull request closes #207 by fixing the behaviour of keyboard focus when using the Tab key on the keyboard.

I also added highlighting to the buttons in the tab titles when they are focused with the keyboard, and added the ability to change the current tab by focusing it with the keyboard and then pressing Enter or Space.

If you want me to update the changelog, let me know what I should put for the version.

Copy link
Owner

@Adanos020 Adanos020 left a comment

Choose a reason for hiding this comment

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

Hi, thanks for looking into this and apologies for the radio silence on the issue.

The code looks sensible, I'll give it a proper test soon (it's 2:37am where I'm at now 😄).

@Adanos020
Copy link
Owner

Switching between widgets with the Tab key seems to work fine.

I noticed that when you try to switch between tabs and their close buttons, it usually takes several presses of the Tab key to get from one tab to the next. I attempted to see if there was something else in focus, and sure enough, the value of ui.memory(|m| m.focus()) changes with each press of the Tab key, even though the switch isn't visible in the UI.

@white-axe Do you have any ideas what might cause that?

I removed focus from two widgets that are not supposed to be focusable.
Also, I added highlighting to the separators when they are focused
because apparently I can't remove the ability to focus them without
breaking the ability to drag the separators.
@white-axe
Copy link
Contributor Author

There were some widgets that were requesting focus when they didn't need it.

I couldn't figure out how to prevent the separators from being focused so for completeness I also added the ability to move them with the arrow keys when focused.

@Adanos020
Copy link
Owner

Adanos020 commented Dec 30, 2023

This is a great improvement! One last thing I think would make this near perfect would be to highlight currently focused separators and tabs the same way they're highlighted on mouse hover. Then I think we can merge this.

Also, if you could update the changelog with the fix and new feature for release 0.10, would be appreciated.

@Adanos020 Adanos020 changed the base branch from main to release-0.10 December 30, 2023 00:08
@white-axe
Copy link
Contributor Author

There's now a visual indication of when an active tab is focused with the keyboard, although it just uses the "focused" tab style because there isn't a unique tab style for tabs that are active and hovered at the same time.

I'm not sure what you mean by highlighting the currently focused separators. I think I already implemented highlighting them when focusing them with the keyboard.

@Adanos020
Copy link
Owner

Adanos020 commented Dec 30, 2023

Hm, yeah, highlighted tabs now look exactly the same as the currently focused tab, i.e. one whose body you've last clicked on.

image

The only exception is when the currently hovered tab is in a node with multiple tabs and it's not the currently selected tab, then it'll have a brighter border. I think all tabs should behave like this, not just "secondary" ones.

image

Now for separators, I think last time I tested I saw the separators wouldn't get highlighted until you attempted to resize one. Although, that might not have actually been the case since now it doesn't behave this way.

This is so that using the arrow keys to change the keyboard focus will
still work normally.
@white-axe
Copy link
Contributor Author

Alright, I've added new styles for tabs that are focused with the keyboard.

Copy link
Owner

@Adanos020 Adanos020 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 a lot for working on this!

@Adanos020 Adanos020 merged commit ed990ed into Adanos020:release-0.10 Dec 31, 2023
4 checks passed
@white-axe white-axe deleted the keyboard-focus branch December 31, 2023 20:37
Adanos020 added a commit that referenced this pull request Jan 8, 2024
* Fix crash after calling `DockState::remove_tab`. (#208)

* Remove surface after removing the last remaining tab, if the surface isn't `Main`.

* Add explicit panic in `Tree::remove_leaf` if the `Tree` is empty.

* Bump patch version, update changelog.

* Include instructions on how to run examples in Readme. (#209)

* Fix keyboard focus not working inside `DockArea`s

* Add visual indicators when tab buttons are focused

* Allow current tab to be switched using the keyboard

* Remove extra focusable areas

I removed focus from two widgets that are not supposed to be focusable.
Also, I added highlighting to the separators when they are focused
because apparently I can't remove the ability to focus them without
breaking the ability to drag the separators.

* Separators can now be moved with the arrow keys

* Add highlighting to active tabs that are keyboard focused

* Update changelog

* Add tab styles for keyboard-focused tabs

* Don't move separators unless Ctrl or Shift is down

This is so that using the arrow keys to change the keyboard focus will
still work normally.

* Clippy

---------

Co-authored-by: Adam Gąsior <adanos020@gmail.com>
@Adanos020 Adanos020 mentioned this pull request Jan 8, 2024
Adanos020 added a commit that referenced this pull request Jan 9, 2024
* Bump crate version

* Keyboard focus fixes (#211)

* Fix crash after calling `DockState::remove_tab`. (#208)

* Remove surface after removing the last remaining tab, if the surface isn't `Main`.

* Add explicit panic in `Tree::remove_leaf` if the `Tree` is empty.

* Bump patch version, update changelog.

* Include instructions on how to run examples in Readme. (#209)

* Fix keyboard focus not working inside `DockArea`s

* Add visual indicators when tab buttons are focused

* Allow current tab to be switched using the keyboard

* Remove extra focusable areas

I removed focus from two widgets that are not supposed to be focusable.
Also, I added highlighting to the separators when they are focused
because apparently I can't remove the ability to focus them without
breaking the ability to drag the separators.

* Separators can now be moved with the arrow keys

* Add highlighting to active tabs that are keyboard focused

* Update changelog

* Add tab styles for keyboard-focused tabs

* Don't move separators unless Ctrl or Shift is down

This is so that using the arrow keys to change the keyboard focus will
still work normally.

* Clippy

---------

Co-authored-by: Adam Gąsior <adanos020@gmail.com>

* Upgrade to egui 0.25 (#214)

* Update readme and changelog

* Add window tooltip translation (#216)

* Add translation for window close button tooltip

* Rename `default` to `english` in `{TabContextMenu,Window,}Translations`

* Update changelog

---------

Co-authored-by: 刘皓 <whiteaxe@tuta.io>
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.

Keyboard focus works with Shift+Tab but not with just the Tab key
2 participants