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 ability to navigate with TAB key #11893

Closed
wants to merge 1 commit into from

Conversation

gillamkid
Copy link
Contributor

Description

There was little to no visual indication to show which buttons where focused when TAB was pressed. I added visual queues and made so the expected inputs would be focused when pressing TAB

Sponsor

This contribution was sponsored by Firestorm
654d4f9476ff2a38f37e9ab9_firestorm-homepage-share-img-2

Demo of new TAB navigating ability (Light mode)

tab-navigation-light.mp4

Demo of new TAB navigating ability (Dark mode)

tab-navigation-dark.mp4

There was little to no visual indication that buttons where focused when TAB was pressed. I added visual queues and made so the expected inputs would be focused when pressing TAB

Contribution Sponsor: Firestorm (launchfirestorm.com)
@DronecodeBot
Copy link

This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there:

https://discuss.px4.io/t/px4-sync-q-a-sep-18-2024/40762/4

@DronecodeBot
Copy link

This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there:

https://discuss.px4.io/t/px4-sync-q-a-sep-18-2024/40762/1

@dagar dagar removed the request for review from HTRamsey September 18, 2024 15:30
@mrpollo
Copy link
Member

mrpollo commented Sep 18, 2024

Hey @HTRamsey can you take a quick look here? I have gone through the changes and they seem fine to me, but another set of eyes wouldn't hurt.

Thank you @gillamkid

@dagar dagar requested a review from HTRamsey September 18, 2024 15:30
@DonLakeFlyer
Copy link
Contributor

I'm still not seeing the videos:
Screenshot 2024-09-18 at 11 56 59 AM

@gillamkid
Copy link
Contributor Author

I'm still not seeing the videos:

Maybe try using Google Chrome on a desktop? I don't see the videos on my phone, but I've tried several computers in Chrome and it works for them all.

@DonLakeFlyer
Copy link
Contributor

What happens with these sorts of changes on mobile? For example if you click a button in a dialog which then does some work? Does that now leave a focus visual on that button on Mobile which doesn't really make sense?

@DonLakeFlyer
Copy link
Contributor

Chrome shows me the videos. I built it myself on OSX but it's not working for me. For example in the instrument value edit dialog it will only move between two text fields in the range section. Strange.

@DonLakeFlyer
Copy link
Contributor

DonLakeFlyer commented Sep 18, 2024

Changing everything from focusPolicy: clickFocus to focusPolicy: tabFocus is going to cause problems. Text fields do validation on loss of focus. If other controls don't have click focus then it doesn't that trigger validation.

Reproduce:

  • Go to App Settings - Fly View
  • Change one of the numeric text fields to "asdf"
  • Click a slider switch to change the enable state
  • With this pull - Field validation doesn't trigger
  • Current QGC master - Field validation triggers

Making sure field validation happens before you close out some part of the ui ends up being tricky. I think this might end up breaking that. I also thinks it's way better to trigger field validation when you click on any other ui control.

@DonLakeFlyer
Copy link
Contributor

DonLakeFlyer commented Sep 18, 2024

A reason for the current state of affairs is that QGC kind of follows a more mobile and or Mac style of keyboard handling. You can tab between text fields but not every single thing. And there is limited support for things like Enter/Esc. It's been my experience that making a good end to end keyboard experience with UI takes a ton of work. It doesn't come for free by creating just any UI. And if you do it half-assed, well it ends up that way and doesn't add much value. I also tend to think all the focus display UI ends up cluttering everything up. Again for not enough value.

If the tabFocus only thing worked then it would be great since I'd assume then the extra focus display would only start showing up if someone actually pressed tab. But without that I'm kind of negative on the whole tab focus thing cluttering up the ui if it's going to be there all the time.

Do you have good reasons why full keyboard support is important?

@gillamkid
Copy link
Contributor Author

I wanted to fix this because TAB support already half works (focus will hop to different elements), it just isn't consistently visually indicated. Also some elements that one would expect to be in the TAB focus chain (namely buttons) could not be tabbed too

Importance: sometimes I like to tab through a form, and I can't in QGC, at least not with the visual focus queues I would like

I'll try to make some fixes so this can be good enough to merge. I've never built for android or macOS, but I'll experiment with that a bit and see if I can get this working on all supported devices

@DonLakeFlyer
Copy link
Contributor

sometimes I like to tab through a form, and I can't in QGC, at least not with the visual focus queues I would like

Ok, but I don't want to support that to the detriment of a crufty UI for all users. Nor do I want to turn QGC into a Windows App with all the UI ridiculousness that goes along with that. I can live with keyboard support and focus indicators like Ubuntu does them. That means they normally don't show. Clicking a control with the mouse doesn't cause them show. They only show up the first time you click tab and start moving around a form.

@DonLakeFlyer
Copy link
Contributor

They only show up the first time you click tab and start moving around a form.

From poking around Qml docs. This means using visualFocus and not activeFocus to determine if focus visuals should show on a control.

@DonLakeFlyer
Copy link
Contributor

Also the focusPolicy needs to be strongFocus which is both tab and click. This fixes the validation on click away problem.

@DonLakeFlyer
Copy link
Contributor

I hacked around with this again last night trying different things. Still no luck getting it to work on Mac. No idea why.

@gillamkid
Copy link
Contributor Author

gillamkid commented Nov 6, 2024

Closing since I don't have immediate plans to finish this. I may take a stab at this again someday.

@gillamkid gillamkid closed this Nov 6, 2024
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.

4 participants