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

Use auto focus on the first input field on all screens #1311

Conversation

blabno
Copy link

@blabno blabno commented Feb 1, 2018

Fixes #1208

fillForm(newValue.getAddressString());
UserThread.execute(() -> amountTextField.requestFocus());
Copy link
Author

Choose a reason for hiding this comment

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

Although UserThread.execute is short I think it's hiding the meaning.
What do you think about creating method in GUIUtil called executeOnNextRenderFrame?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm... Maybe better if we rename "execute". But that is used in many places so I would prefer to do that myself to check all and have an atomic commit with only that refactoring.

Copy link
Author

Choose a reason for hiding this comment

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

I think it's fine to have an additional method in proper place (GUIUtil) that simply delegates to UserThread if that method's name tells us what's the purpose. Just in case impl changes later.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. We should use that only then for the render in next frame use case as that is GUI specific. The UserThread.execute is GUI agnostic and works for headless versions as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually I am not sure if the UserThread.execute is needed at all cases. Can you test the different views if it works without the UserThread.execute as well? I will wait for merging the PR until that is resolved.

Copy link
Author

@blabno blabno left a comment

Choose a reason for hiding this comment

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

Question

@blabno blabno force-pushed the feature/1208-use-auto-focus-on-the-first-input-field-on-all-screens branch from 14f08a6 to 53a7ff9 Compare February 1, 2018 09:17
@blabno
Copy link
Author

blabno commented Feb 1, 2018 via email

@ManfredKarrer
Copy link
Contributor

Ok, then I would suggest to use GUIUtil.requestFocus(Node node).

@blabno blabno force-pushed the feature/1208-use-auto-focus-on-the-first-input-field-on-all-screens branch from 53a7ff9 to f0a985e Compare February 3, 2018 11:31
@blabno
Copy link
Author

blabno commented Feb 3, 2018

Introduced GUIUtil.requestFocus.

Copy link
Contributor

@ManfredKarrer ManfredKarrer left a comment

Choose a reason for hiding this comment

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

Seems all ok to me.

@ManfredKarrer ManfredKarrer merged commit 1bd222c into bisq-network:master Feb 3, 2018
cbeams added a commit that referenced this pull request Feb 8, 2018
This commit reverts a change added in
f0a985e (#1311) that made it impossible
to scroll through arbitration tickets with the arrow keys, because on
selecting each dispute/ticket, the input text area would steal focus
away.

Now, the input text field still gets auto-focused when the dispute view
is opened, but does not try to steal / auto-focus when scrolling through
/ selecting individual disputes.
@cbeams
Copy link
Contributor

cbeams commented Feb 8, 2018

@blabno, @ManfredKarrer, there was a minor bug introduced with these changes. I've just fixed it in 8d44f75.

@cbeams cbeams added this to the v0.6.6 milestone Feb 8, 2018
@ManfredKarrer ManfredKarrer removed this from the v0.6.6 milestone Feb 23, 2018
@blabno blabno deleted the feature/1208-use-auto-focus-on-the-first-input-field-on-all-screens branch April 6, 2018 09:18
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.

3 participants