Focus project-find search input upon toggle #645
Conversation
| @@ -72,6 +72,7 @@ module.exports = | |||
| @projectFindPanel.hide() | |||
| else | |||
| @projectFindPanel.show() | |||
| @projectFindView.focusFindElement() | |||
There was a problem hiding this comment.
Rather than just adding the one missing line, maybe we could turn these two implementations into a common function so that find and find-in-project always work the same?
There was a problem hiding this comment.
Sure, that would make sense. I guess in that case we can do the same for other functions like show. Most of these functions seem to share some actions:
- Create the view.
- Disable 'the other find panel'.
- Show/hide the correct find panel.
- Optionally focus a specific element.
I would propose the following changes:
- Write a function that determines 'the other find panel' based on the panel to be activated.
- Generalize functions so that they take a panel to activate and an optional action like 'focus input x' as arguments.
What do you think? I'll try to write some more code soonish.
|
I've generalized the |
| @findPanel.hide() | ||
| @projectFindPanel.show() | ||
| @projectFindView.focusFindElement() | ||
| showPanel @projectFindPanel, @findPanel, @projectFindView.focusFindElement |
There was a problem hiding this comment.
👕 Some extra whitespace at the end of this line
|
How sloppy... |
|
Do these functions need unit tests? |
|
It's preferable if you can add unit tests for these, yes. That way we can be relatively certain we don't accidentally break them as things move forward. |
|
I will try to figure out how to write these UI tests based on the existing tests for the package. Thanks @lee-dohm. |
Focus project-find search input upon toggle
|
Thanks! |
|
But the tests were still missing o_O |
|
It would be great if you could submit a separate PR for the tests 😀 |
|
Ahahahah, will try. Thanks. |
This PR adds the following change: when toggling the project-find panel via the
project-find:togglecommand from an invisible state, the search input is focused after the panel becomes visible. This is preferable for the following reasons:find-and-replace:togglecommand. Enabling the same behaviour forproject-find:togglemakes find-and-replace more consistent from a UX perspective.Escand create according key bindings in theirkeymap.cson. In such cases it's convenient to also focus the search input.