Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

More app menu work-in-progress #84

Merged
merged 6 commits into from
Dec 23, 2015
Merged

More app menu work-in-progress #84

merged 6 commits into from
Dec 23, 2015

Conversation

diracdeltas
Copy link
Member

This started to conflict with the windowstore/appstore changes, so I'm opening it now.

Auditors: @bbondy

@@ -145,7 +145,10 @@ class UrlBar extends ImmutableComponent {
}

componentWillMount () {
ipc.on(messages.SHORTCUT_FOCUS_URL, this.onFocus.bind(this))
ipc.on(messages.SHORTCUT_FOCUS_URL, () => {
this.onFocus.bind(this)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, this is wrong and causing test failures. Will fix!

@diracdeltas
Copy link
Member Author

cc @bridiver since tests seem to be failing inconsistently. travis reports:

  25 passing (26s)
  2 failing
  1) urlbar change tabs tab with typing clears the selected text:
      undefined: expected 'about:blank' to equal ''
      + expected - actual
      -about:blank


  2) urlbar change tabs switch to typing tab preserves typing state:
      undefined: expected 'about:blank' to equal 'a'
      + expected - actual
      -about:blank
      +a

Locally when I run npm test, I never see the first error and I only see the second one sometimes. Any ideas?

* @param {Array} message message and arguments to send
* @return {boolean} whether the message was sent
*/
const sendToFocusedWindow = (focusedWindow, message) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks I've been wanting to do this, so much cleaner.

@bbondy
Copy link
Member

bbondy commented Dec 23, 2015

Thanks, feel free to merge.
Nice to have the open / save.

@diracdeltas
Copy link
Member Author

Merging for now despite Travis since I have another branch that depends on this.

diracdeltas added a commit that referenced this pull request Dec 23, 2015
@diracdeltas diracdeltas merged commit d4e931a into master Dec 23, 2015
@bbondy
Copy link
Member

bbondy commented Dec 23, 2015

np, thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Command+L does not select URL bar text when URL bar is focused. It should.
2 participants