Fixes for focus handling

[gruehle]

@peterflynn

This pull request has a couple key changes to the way focus is handled and managed.

• EditorManager.focusEditor() now returns focus to the last visible
  editor that had focus. We no longer force the focus back to the
  main editor after saving or cancelling an open.
• Add EditorManager.getActiveEditor(). This function is similar to
  getFocusedEditor(), but will return the last editor that was given
  focus, 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.
• Update FindReplace, FindInFiles and QuickOpen to use getActiveEditor()

I tested the changes in the following scenarios:

• File Save - If focus is in an inline editor, focus is returned to the editor after saving (logged as issue When saving changes in an inline editor, focus should not be set to the main editor. #547)
• File Open - if the open is cancelled, focus is not forced to the main editor
• Changing indent size - focus is returned to the editor that previously had focus
• Find/Replace - Cmd-G works while the find dialog is open (issue Should be able to "find next" while focus is in the Find bar #1935)

[peterflynn]

I noticed one odd behavior:

1. Open Quick Open
2. Use the shortcut to open Find, or Find in Files
3. Type something and hit enter

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.

[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).

[peterflynn]

Another issue:

1. Type in the search bar
2. Hit the shortcut for Find Next several times
3. Press Enter

Selection jumps back to the first search result.

[peterflynn]

Could we still include this bit: "If no editor visible, does nothing."

[gruehle]

Done

[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.

[peterflynn]

Actually one last thing: I think we should adjust the "focusedEditorChange" event docs at the top of EditorManager too.

• Change "Roughly, this event tracks getFocusedEditor() changes" --> "This event tracks getActiveEditor() changes"
• Change the TODO warning at the bottom... maybe note that getFocusedEditor() isn't always in sync with the event, and event handlers should use their args or check getActiveEditor() instead.

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.

[gruehle]

I ended up making the focusedEditorChange -> activeEditorChange rename you suggested. It does make things cleaner.

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

[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).

[peterflynn]

Oops, crud -- I made all my comments on your commit 5817238 instead of the pull request's main diff. Sorry!
Done re-reviewing, though.

[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.

[gruehle]

@peterflynn - I pushed fixes for the Find and Quick Open bugs. For Quick Open it ended up being a bit more difficult because we don't want to block mouse events in the Search field, but we do want to block events in the popup. I ended up keeping the existing _close() code for safety--it may not be needed, but doesn't seem like it's worth the testing effort to remove.

@jasonsanjose - Ugh, that is an ugly bug. Do you know if it is filed? Since these changes don't make things any worse, I'm going to say that it is out of scope for this story. The former focusedEditorChange event wouldn't work in this case because it was only sent when an editor gained focus, but not when it lost focus.

[jasonsanjose]

Filed #2064.

[peterflynn]

Unit tests look good enough to me. I'll code review the last commit at 2:30 pm, after my string of meetings.

[peterflynn]

Alright, looking good -- merging