Improved the way the search flyout is behaving. #826
Conversation
The search command will now show the flyout and focus on the textbox. If the textbox has no focus and search command is used again it will focus the textbox. A new command has been added to close the search flyout (escape by default) which can now be used to close it.
| @@ -283,6 +283,16 @@ public ICollection<KeyBinding> GetDefaultKeyBindings(Command command) | |||
| } | |||
| }; | |||
|
|
|||
| case Command.CloseSearch: | |||
There was a problem hiding this comment.
I don't think we need an extra command for this. I think esc is intuitive enough
There was a problem hiding this comment.
Yes I thought so too but the issue seems to be that when xterm.js has focus it "steals" keyboard processing. This was my quick idea to avoid this and still receive notifications about keypresses. Am I wrong? If so then do you have some other idea how to do this? :)
There was a problem hiding this comment.
After thinking about it I think we shouldn't care about escape if the search has no focus. Browser seem to handle it similar (escape is not closing the search if a textarea has focus)
But the other changes sound great 👍
There was a problem hiding this comment.
I have checked the browsers and also some IDEs and here is a summary.
These behave the way new implementation handled things (which is CTRL+F fires search/focuses if active, esc closes it regardless of focus):
- Visual Studio 2019
- VS Code
- Chrome
- Edge
- Eclipse
Now here is a list of apps that don't care about the esc press when not in focus:
- Firefox
;p
So as you can see the standard behavior of most advanced and well known apps which employ this feature is the one I've proposed. If you still think we can skip the esc part then I will remove it later. So what do you think? :)
There was a problem hiding this comment.
You convinced me 😄
Sorry for the long wait
The search command will now show the flyout and focus on the textbox. If the textbox has no focus and search command is used again it will focus the textbox.
A new command has been added to close the search flyout (escape by default) which can now be used to close it if the app has focus.
Previous behavior is somewhat odd compared to how nearly all other apps are handling it.