-
Notifications
You must be signed in to change notification settings - Fork 0
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
[CLOSED] Fixes for focus handling #1960
Comments
Comment by peterflynn I noticed one odd behavior:
In both cases, the Quick Open dialog stays open. If you use Find, the dialog gets semi-taken over by Find's event listeners: typing will both select text in the file and narrow down the Quick Open popup. If you use Find in Files, it looks like the find bar comes up underneath the Quick Open bar: the Quick Open field loses focus, but if you type something and hit Enter it'll kick off a search. |
Comment by peterflynn Another issue, possibly puntable: if I launch Find while on the last stage of the Replace "wizard," the Replace bar closes but the Find bar does not open. (At earlier stages of Replace, Find does open). |
Comment by peterflynn Another issue:
Selection jumps back to the first search result. |
Comment by peterflynn Done reviewing. Code looks good other than that one comment nit. I think we should probably fix the 1st & 3rd bugs I noticed, but I don't feel like we need to fix the 2nd right now. |
Comment by peterflynn Actually one last thing: I think we should adjust the "focusedEditorChange" event docs at the top of EditorManager too.
Or we could go whole-hog and rename "focusedEditorChange" to "activeEditorChange"... but that's a slight API break. Might be worth it for clarity, though. There probably aren't many people using it yet given that before Sprint 15 it didn't work reliably. |
Comment by gruehle I ended up making the I pushed hacky fixes for both of the bugs you mention. I'm open to suggestions on better fixes, but since the UI for both of these is temporary, it didn't seem like it was worth it to make broader changes to the way those dialogs are managed |
Comment by gruehle One more thing - can you think of any other unit tests that should be added? I added a focus check to the inline editor save tests (which is the most important thing). |
Comment by peterflynn Oops, crud -- I made all my comments on your commit 5817238 instead of the pull request's main diff. Sorry! |
Comment by jasonsanjose Trying to get ahead of the curve on scenario testing. I tried setting a selection on an editor, blurring away (clicking on the bg of the working set), then trying a number of commands like Edit > Indent. These are no-ops since there is no focused editor. For someone who doesn't recognize that CM has lost focus, it's hard to tell why this menu item doesn't work. This behavior is already existing on master. This is probably out of scope for this user story, but it seems like we would still need a focusedEditorChange-ish event for this use case where we would probably disable the menu items. |
Comment by gruehle
|
Comment by jasonsanjose Filed #2064. |
Comment by peterflynn Unit tests look good enough to me. I'll code review the last commit at 2:30 pm, after my string of meetings. |
Comment by peterflynn Alright, looking good -- merging |
Issue by gruehle
Friday Nov 02, 2012 at 00:20 GMT
Originally opened as adobe/brackets#2026
@
peterflynnThis pull request has a couple key changes to the way focus is handled and managed.
EditorManager.focusEditor()
now returns focus to the last visibleeditor that had focus. We no longer force the focus back to the
main editor after saving or cancelling an open.
EditorManager.getActiveEditor()
. This function is similar togetFocusedEditor()
, but will return the last editor that was givenfocus, even if it doesn't have focus at the moment. This is useful
for commands like Find/Find Next that should work on the active
document even if the Find dialog has focus.
getActiveEditor()
I tested the changes in the following scenarios:
gruehle included the following code: https://github.com/adobe/brackets/pull/2026/commits
The text was updated successfully, but these errors were encountered: