-
-
Notifications
You must be signed in to change notification settings - Fork 491
Dialog focus #1472
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
Dialog focus #1472
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not working for me. When I try to upload the dialog opens but the <input> does not have the focus.
dialog-focus.mp4
232ab6d to
09f7b1a
Compare
@kittaakos Thank you for spotting it. Please try it again, it should work correctly now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not work for me. Once the focus is removed, IDE2 does not put back the focus back to the <input> on dialog open.
focus.mp4
I have not checked the button focus changes yet.
It does not work for me. Once the focus is removed, IDE2 does not put back the focus back to the on dialog open.
You're right, it works only the first time. I've fixed it now, thanks for spotting it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting the focus on the <input> field works great in the user field dialog. Thank you!
I have noticed this difference between 2.0.0 and build from this PR. The first button has a bold border for the firmware updater and the SSL certificate dialogs.
2.0.0:
Screen Shot 2022年09月29日 at 13 27 22
Build from PR:
Screen Shot 2022年09月29日 at 13 27 32
2.0.0:
Screen Shot 2022年09月29日 at 13 27 47
Build from PR:
Screen Shot 2022年09月29日 at 13 28 05
#1373 is not mentioning this change, If this is what we want, please proceed with the merge.
@kittaakos those borders are bold because the focus goes on them when the dialog shows up. #1373 only mentions the user fields dialog, but I wanted to solve the issue more broadly. Thanks for the review!
but I wanted to solve the issue more broadly.
Can you explain why did you decide to set the focus on the buttons only in these two dialogs and not on the others?
Can you explain why did you decide to set the focus on the buttons only in these two dialogs and not on the others?
My reasoning was to try to set the focus on the most appropriate UI element in the dialog. Of course, the definition of most appropriate is arbitrary, so I did what I thought it was best.
Here's the reasoning I've applied on each dialog:
- the user fields one needs to have the focus on the first input element because when the user opens it, the will need to fill them in in order to upload
- with the firmware updater one, the first action the user would like to take is to press the button (if a board is found) because a board is automatically selected
- when the user opens the SSL root certificate one, it's usually not possible to click on the UPLOAD button because if you don't add a certificate before it will be disabled, so I've set the focus on the first button
So, generally I guess my rationale was to set the focus on the UI element that lets the user do what they want to do with a dialog as quickly as possible.
Of course, all of this is questionable, so if you have suggestions or if you disagree with these decisions, please let me know!
So, generally I guess my rationale was to set the focus on the UI element that lets the user do what they want to do with a dialog as quickly as possible.
Thank you for writing down the explanation.
Motivation
Some dialogs, when being opened, don't set the focus on the controls (inputs, button, etc)
Change description
Fixed the focus in the following dialogs:
Also changed "Configure And Upload" label to "Configure and Upload".
Other information
Fixes #1373
Reviewer checklist