-
Notifications
You must be signed in to change notification settings - Fork 973
Bloodhound for URL bar suggestions & move suggestions to browser process #8824
Conversation
243f473
to
e10c2ab
Compare
5a256f2
to
6a4d3f3
Compare
2010302
to
abddb25
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
When you arrow over suggestions which don't match the current URL format, their URL isn't put into the URL bar. After talking to you, this is expected... but it feels a little awkward. If you hit enter, it does work as expected though
edit: this was already captured with URL is not changing in Address bar #8306 -
I think results should prefer the URL over the title / other criteria. Here's an example:
None of the results are for www.google.com
. However, if I type the protocol in:
this looks as expected
js/actions/appActions.js
Outdated
/** | ||
* Indicates that the urlbar text has changed, usually from user input | ||
* | ||
* @param {string} location - The text to set as the new navbar URL input |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docs need updating to include the windowId / input fields
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
derp- this is an individual commit, will go back to looking at the all files changed view 😛 Disregard (you had already fixed it!)
I think this PR fixes this issue: #5878 |
The weird behavior I noticed with TAB has been logged as a separate issue: #8919 (happens in master- not caused by this PR) |
Also fixes various other failing unit tests
- search suggestions failing test - tab tests tab transfer can detach into new windows
This makes results appear much better now and fixes the various use cases you found
Fix #5878 Auditors: @bsclifton
Auditors: @bsclifton Fix #5878 Test comming in a follow up commit now, just pushing without so you can get a build going
Matches things better if the user puts a / in the input
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are some great improvements! Great job 😄
Definitely faster- the results are more relevant / matching, and you fixed some related bugs too
++++!
Bloodhound for URL bar suggestions & move suggestions to browser process
Bloodhound for URL bar suggestions & move suggestions to browser process
This PR does several things:
Fix #7453
Fix #5878
Fix #8919
Submitter Checklist:
Test Plan:
Reviewer Checklist:
Tests