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

SingleTextInputDialog handle the enter key when mouse in the input box #5868

Merged
merged 1 commit into from
Aug 7, 2019
Merged

SingleTextInputDialog handle the enter key when mouse in the input box #5868

merged 1 commit into from
Aug 7, 2019

Conversation

xieyuanhuata
Copy link
Contributor

@xieyuanhuata xieyuanhuata commented Aug 6, 2019

fix #5867
SingleTextInputDialog handle the enter key only when mouse in the input box

before
11

after
good

How to test:
Open 'New Folder' or 'Rename File' and press 'enter' in the space;

Copy link
Contributor Author

@xieyuanhuata xieyuanhuata left a comment

Choose a reason for hiding this comment

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

Only work when the mouse is in the input box

@vince-fugnitto
Copy link
Member

@xieyuanhuata
Copy link
Contributor Author

xieyuanhuata commented Aug 6, 2019

@vince-fugnitto
PR #5865

What it does
fix #5859 - stack of dialogs is tracked and keyboard events are applied only to the top-most dialog from the single keyboard listener for the document body
closes #5860 - superseded

so i have created a new PR

@akosyakov akosyakov added the shell issues related to the core shell label Aug 6, 2019
@vince-fugnitto
Copy link
Member

so i have created a new PR

Should the other PR be closed then?
The code for handleEnter is identical in both PRs #5868 and #5860.

@xieyuanhuata
Copy link
Contributor Author

@vince-fugnitto
Do I need to clean up the same code of PR #5860 ?

@vince-fugnitto
Copy link
Member

@vince-fugnitto
Do I need to clean up the same code of PR #5860 ?

I was just trying to understand why another PR was opened with a subset of the first PRs changes.
I think a single PR with the correct changes is sufficient.

@akosyakov
Copy link
Member

akosyakov commented Aug 6, 2019

@vince-fugnitto I've closed #5860, applying of keyboard listener only to the top level dialog is handled in #5865. I think this PR handles another issue, that only input element should be focused.

Could you review #5865 as well please?

@xieyuanhuata
Copy link
Contributor Author

@vince-fugnitto @akosyakov
Could you tell me where I need to change? Thank you.

@vince-fugnitto
Copy link
Member

@vince-fugnitto @akosyakov
Could you tell me where I need to change? Thank you.

It's alright, @akosyakov closed the original PR.

@xieyuanhuata
Copy link
Contributor Author

xieyuanhuata commented Aug 6, 2019

@vince-fugnitto
For 'apply overlay keyboard events to the top most dialog',Mr akosyakov think my change is not perfect ,so he commited PR #5865

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

The code works well 👍

If the input is not focused enter does not execute the dialog.
Only if the input is selected does enter work.

@akosyakov akosyakov merged commit f48398e into eclipse-theia:master Aug 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
shell issues related to the core shell
Projects
None yet
3 participants