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

Focus webview when ESC pressed on UrlBar #7039

Closed

Conversation

michalbe
Copy link
Contributor

@michalbe michalbe commented Feb 3, 2017

Fixes #3248

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

Test Plan:

  • Focus the URL bar
  • Press ESC
  • notice the focus returned to the webview

Copy link

@cndouglas cndouglas left a comment

Choose a reason for hiding this comment

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

Thanks! Works for me.

@cndouglas cndouglas added this to the 0.13.3 milestone Feb 4, 2017
@bbondy bbondy removed this from the 0.13.3 milestone Feb 4, 2017
@bbondy
Copy link
Member

bbondy commented Feb 4, 2017

Sorry this is not the correct behaviour and would diverge us from how all other browsers work. We're all about being different for our differentiators, but for standard UX things that people expect to work one way, and have trained themselves to think they should work that way, we shouldn't diverge. In particular browser users often press esc to clear what they are typing and revert the URL text to the previous one.

@michalbe
Copy link
Contributor Author

michalbe commented Feb 4, 2017

@bbondy I understand your point and it makes perfect sense to me, but I think things like this should be pointed out in the original issue spec (in #3248) before it was marked as good-first-bug and feature request half a year ago. There's 1.8k+ opened issues and it's hard for non-employees to follow your vision of the project when those tasks are not triaged properly. Anyway - I'm impressed with how Brave is growing, I love what you guys are doing and I hope sooner or later I will find an issue that's valid and properly scoped :). Thanks, and have a nice weekend.

@bbondy
Copy link
Member

bbondy commented Feb 4, 2017

@michalbe definitely, and I'm sorry you had to spend time on it and we didn't realize it earlier. I don't have full visibility over all issues but we try to do our best to catch things at the issue level. Most issues are fine that are open, but feel free to hit me up to make 100% sure in the issue if you do another. Thanks for taking the time to contribute and sorry we dropped the ball on this one.

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

Successfully merging this pull request may close these issues.

4 participants