[CLOSED] prevent event from being used to enter a newline into editor

[core-ai-bot]

Issue by zaggino
Friday Apr 11, 2014 at 22:36 GMT
Originally opened as adobe/brackets#7493

---

this fixes brackets-userland/brackets-git#385
cc@TomMalbran@peterflynn

---

zaggino included the following code: https://github.com/adobe/brackets/pull/7493/commits

[core-ai-bot]

Comment by peterflynn
Monday Apr 14, 2014 at 17:50 GMT

---

@njx Per earlier conversation, I think the reason this doesn't crop up in our dialogs much is that most of our dialogs fail to re-focus the editor when they're being closed -- so the subsequent keyup doesn't go the editor, it just goes off into space. Whenever we fix our dialogs to behave better I think we'd need this fix for core too :-)

[core-ai-bot]

Comment by njx
Wednesday May 14, 2014 at 18:57 GMT

---

Sorry this missed release 39. I'm tagging it for release 40 so I don't forget about it.

[core-ai-bot]

Comment by njx
Wednesday May 21, 2014 at 23:19 GMT

---

@zaggino Just pushed an update to this that does the stop/preventDefault in more cases. Would you like to review it? I did some testing with various dialogs in brackets-git as well as the file filters dialog and everything seems to be working okay, but it could probably use more testing.

@peterflynn FYI - do you think this is reasonably safe?

[core-ai-bot]

Comment by MarcelGerber
Friday May 23, 2014 at 18:56 GMT

---

@njx Writing spaces into the Extension Manager search box is very laggy with this PR.

[core-ai-bot]

Comment by njx
Friday May 23, 2014 at 19:41 GMT

---

@SAPlayer Are you sure that's specific to this branch? Any lag there is probably from the actual search through the large number of extensions taking time, not from the event handling. I don't notice any difference between master and this branch in that regard.

[core-ai-bot]

Comment by MarcelGerber
Friday May 23, 2014 at 19:57 GMT

---

Oh, you're right. I'm sorry.

[core-ai-bot]

Comment by redmunds
Tuesday Jun 03, 2014 at 22:58 GMT

---

Change Milestone to Release 0.41

[core-ai-bot]

Comment by njx
Tuesday Jun 03, 2014 at 23:27 GMT

---

Assigning to@zaggino to review my changes to the original PR.

[core-ai-bot]

Comment by zaggino
Wednesday Jun 04, 2014 at 00:27 GMT

---

@njx I had a problem when another button than primary was focused (Close button) but pressing enter key would still launch click handler for the primary button. See adobe/brackets@046aa94

[core-ai-bot]

Comment by zaggino
Wednesday Jun 04, 2014 at 00:32 GMT

---

And I'm not 100% sure whether to use $focusedElement or this.find(e.target) there because $focusedElement seems to consider only elements with class dialog-button ...

[core-ai-bot]

Comment by njx
Friday Jun 06, 2014 at 23:49 GMT

---

@zaggino Interesting - this looks like a bit of a difference between Mac and Windows. In native Mac apps (if "Full Keyboard Access" is on, so you can tab to buttons), Enter always activates the default button even if a different button has focus - you have to hit Space to activate the focused button. On Windows, when a button is focused, both Enter and Space activate it.

(The Windows behavior actually has a slight added nuance. If a non-button control in the body of the dialog is focused, then the original primary button has the "primary button" highlight, and hitting Enter will activate it. But if a button control anywhere in the dialog is focused - regardless of whether it's a dismissal button - then that button control gets both the focus look and the "primary button" look, making it clearer that both Enter and Space will activate it. So the "primary button" highlight actually jumps around as you tab through buttons in dialogs, then returns to the original primary button whenever a non-button is focused.)

I could imagine keeping the original behavior on Mac and changing it for Windows only - though I don't really know how many people are familiar with this nuance on the Mac (perhaps some people might turn it on for accessibility reasons). Or we could just decide not to deal with all these nuances and just keep what you have on both platforms (i.e., if the focused element is a button, Enter always activates it, otherwise activates the original primary button, and we don't worry about moving the "primary button" highlight around).

@larz0 do you have an opinion?

[core-ai-bot]

Comment by larz0
Saturday Jun 07, 2014 at 00:36 GMT

---

@njx I think we should just keep what@zaggino has on both platforms and see how that goes.

[core-ai-bot]

Comment by zaggino
Wednesday Jun 11, 2014 at 23:06 GMT

---

@njx
Didn't find any more issues with this, I think it's good to merge if no-one objects.

[core-ai-bot]

Comment by njx
Thursday Jun 12, 2014 at 02:03 GMT

---

Sounds good - I will try to look at your most recent change tomorrow and get it merged.

[core-ai-bot]

Comment by njx
Thursday Jun 12, 2014 at 21:18 GMT

---

Looks good, merging. Sorry this took so long.