Search Settings Panel: Search Suggestions (Part 1) #2726

darkdh · 2016-07-26T14:42:31Z

Submitted a ticket for my issue if one did not already exist.
Used Github auto-closing keywords in the commit message.
Ran git rebase -i to squash commits if needed.

darkdh · 2016-07-27T01:33:14Z

The cursor will be point on each entry to indicate user that is clickable. Recording cannot catch it and I don't know why.

bbondy · 2016-07-27T17:36:06Z

wow this is looking really awesome so far. The functionality is a major improvement over our current search experience.

Here's a few things I noticed from testing:

When I go to Search preferences, it is not showing me my default from 0.11.1
It might already be the case but please make sure the default from 0.11.1 is preserved
When you detect a prefix in the urlbar please show the favicon to the left in the urlbar.
The titles of the search preferences page (Default, Search Engines, Engine Go Key (type first)), seem to underline and look like they are clickable but they don’t do anything. Maybe just remove the underlining for them for now.

bbondy · 2016-07-27T17:38:06Z

docs/state.md

@@ -134,7 +134,7 @@ AppStore
    'general.downloads.default-save-path': string, // default path for saving files
    'general.autohide-menu': boolean, // true if the Windows menu should be autohidden
    'general.disable-title-mode': boolean, // true if title mode should always be disabled
-    'search.default-search-engine': string, // path to the open search XML
+    'search.default-search-engine': string, // name of search engine, from js/data/searchProviders.js


This will need to be set inside app.sessionStore based on the user's old selected xml engine. And only done once, after that it should jus preserve the setting. Also some users won't have an old xml setting if it is their first time install.

It is the same setting name as before so just check for special case values of content/search/google.xml and content/search/duckduckgo.xml when loading the session state and migrate accordingly.

bbondy · 2016-07-27T17:44:52Z

ok made a few minor comments but this is close to being able to be merged, this might still make 0.11.2.

bbondy · 2016-07-29T03:11:42Z

Pls also add some tests for the shortcut prefix text for this as well.

bbondy · 2016-07-29T03:11:52Z

thanks!

darkdh · 2016-07-29T03:44:57Z

Ok, working on it

bbondy · 2016-07-29T14:37:48Z

There's a couple things off about the urlbar icon image. It seems like not enough padding on the right and also some of the favicons just look bad. cc @bradleyrichter please take a look locally before you work more on that aspect @darkdh

bbondy · 2016-07-29T14:38:41Z

app/sessionStore.js

+      }
+      if (data.settings['search.default-search-engine'] === 'contnt/search/duckduckgo.xml') {
+        data.settings['search.default-search-engine'] = 'DuckDuckGo'
+      }


bradleyrichter · 2016-07-29T14:45:12Z

ok. I'll run the PR and reply with suggestions if needed.

So far from the clip above, it looks great!

bbondy · 2016-07-29T14:46:07Z

js/components/urlBar.js

+          const searchRE = new RegExp('^' + entry.shortcut + ' .*', 'g')
+          if (searchRE.test(location)) {
+            const iconSize = 16
+            this.searchFaviconStyle = {


Style should really only be calculated inside render, here you'd set state which indicated the active engine, something like frame.x.navbar.urlbar.activeSearchEngine. I think we should also show the favicon as soon as you put searchTerm + ' ' and you don't need an extra char after that. I think it will be fixed by what I'm suggesting here.
You might want to wait for Brad to weigh in first to make sure we still want to have the favicon in the urlbar because I mentioned some of the favicons are looking bad as is, like yahoo and duck duck go seems pixelated.

bbondy · 2016-07-29T14:54:48Z

It's a super great feature, can't wait to get it in. My concerns are with some favicon replacement in the urlbar after the shortcut is entered, in particular I noticed duckduckgo looks small and pixelated and yahoo icon looks huge and ugly. Text doesn't seem to be fully aligned and missing padding as small tweaks that would help. Maybe we need a smaller than 16x16 image here.

darkdh · 2016-07-29T14:59:23Z

I noticed that not just urlbar but in search engine table got the same issue. Maybe we should replace the icon image source

luixxiul · 2016-07-29T14:59:55Z

It would be nice if svg images can be used here

bradleyrichter · 2016-07-29T16:55:59Z

There is a mystery to solve as to why they appear different in the URL bar and tab...

@darkdh Can you check if the same favicon path is being used in both cases?

Can you set the URL bar icon to appear after the space is typed rather than waiting for the first letter?
It looks like the URL bar favicon needs to move down 2 px.
Make sure it is hidden when the URL bar changes to Title Mode.

oops. the wrong favicon is sticking around in different URLS.

bradleyrichter · 2016-07-29T17:43:31Z

@darkdh I restyled the panel a bit but I won't commit until your are done with the other things.

Please remove the title under "Search Settings" in your code though...

luixxiul · 2016-07-29T17:46:43Z

Are favicons cached here?

darkdh · 2016-07-30T08:48:18Z

Thanks for all your valuable feedback.
There is one problem remain that cause the different icon between preference and urlbar.
Which is I cannot use favicon url in preference due to CSP.

So in 3efd6dc there are two image source of search providers in js/data/searchProviders.js. Static data uri is for preference used and favicon url can be used in urlbar.

bbondy · 2016-07-31T01:45:07Z

cc @diracdeltas for advise.

diracdeltas · 2016-08-01T23:44:10Z

feel free to change the app/extensions.js CSP (i assume that's where the error is from?) to * data:

darkdh · 2016-08-02T01:47:50Z

Thanks @diracdeltas. It works fine now.

bradleyrichter · 2016-08-02T01:53:40Z

@darkdh Can you please use this fontawesome icon in place of the "X" to indicate the default choice?

http://fontawesome.io/icon/check-square/

16x16px

Color it as @braveorange or #FF5000.

darkdh · 2016-08-02T01:56:06Z

@bradleyrichter Ok, I will replace it with fontawesome in next commit.

darkdh · 2016-08-02T03:06:41Z

@bradleyrichter , please take a look at screenshot of b9567d6

bradleyrichter · 2016-08-02T03:48:57Z

looks great! thanks...

fix brave#2694

bbondy · 2016-08-02T05:39:08Z

thanks for all your hard work on this!

bbondy added this to the 0.11.3dev milestone Jul 26, 2016

darkdh force-pushed the SearchSuggetions branch from 5af84f9 to f7fa659 Compare July 27, 2016 15:06

darkdh changed the title ~~Search Settings Panel: Search Suggestions (Part 1) [WIP]~~ Search Settings Panel: Search Suggestions (Part 1) Jul 27, 2016

darkdh force-pushed the SearchSuggetions branch from 0359c3a to 99019b3 Compare July 27, 2016 15:39

bbondy reviewed Jul 27, 2016
View reviewed changes

luixxiul modified the milestones: 0.11.2dev, 0.11.3dev Jul 28, 2016

bbondy modified the milestones: 0.11.3dev, 0.11.2dev Jul 28, 2016

bbondy reviewed Jul 29, 2016
View reviewed changes

bbondy modified the milestones: 0.11.2dev, 0.11.3dev Aug 2, 2016

darkdh added 9 commits August 2, 2016 13:21

Search Settings Panel: Search Suggestions (Part 1)

82997f9

fix brave#2694

xml migration and go-key favicon indication

a47ee1a

Save select entry for reducing search time

f392de0

Clear icon when escape & add test

15dcc1f

Fix search engine detection

3468242

Allow img-src in CSP

bf9f54f

Fontawesome check-square

820443f

Fix backspace detection and icon margin

e9ba607

Replace & with &

688fa23

darkdh force-pushed the SearchSuggetions branch from 6fa97b1 to 688fa23 Compare August 2, 2016 05:29

bbondy merged commit 2e150a4 into brave:master Aug 2, 2016

darkdh deleted the SearchSuggetions branch August 2, 2016 05:39

luixxiul mentioned this pull request Aug 6, 2016

Localization of about:preferences#search #3004

Closed

luixxiul mentioned this pull request Sep 28, 2016

Create an additional (optionally) smaller search field for the navigation toolbar #466

Closed

luixxiul added the feature/search label Oct 5, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Search Settings Panel: Search Suggestions (Part 1) #2726

Search Settings Panel: Search Suggestions (Part 1) #2726

darkdh commented Jul 26, 2016 •

edited

Loading

darkdh commented Jul 27, 2016 •

edited

Loading

bbondy commented Jul 27, 2016

bbondy Jul 27, 2016

bbondy Jul 27, 2016

bbondy commented Jul 27, 2016

bbondy commented Jul 29, 2016

bbondy commented Jul 29, 2016

darkdh commented Jul 29, 2016

bbondy commented Jul 29, 2016

bbondy Jul 29, 2016

bradleyrichter commented Jul 29, 2016

bbondy Jul 29, 2016

bbondy commented Jul 29, 2016

darkdh commented Jul 29, 2016

luixxiul commented Jul 29, 2016

bradleyrichter commented Jul 29, 2016

bradleyrichter commented Jul 29, 2016

luixxiul commented Jul 29, 2016

darkdh commented Jul 30, 2016

bbondy commented Jul 31, 2016

diracdeltas commented Aug 1, 2016

darkdh commented Aug 2, 2016

bradleyrichter commented Aug 2, 2016

darkdh commented Aug 2, 2016

darkdh commented Aug 2, 2016

bradleyrichter commented Aug 2, 2016

bbondy commented Aug 2, 2016

Search Settings Panel: Search Suggestions (Part 1) #2726

Search Settings Panel: Search Suggestions (Part 1) #2726

Conversation

darkdh commented Jul 26, 2016 • edited Loading

darkdh commented Jul 27, 2016 • edited Loading

bbondy commented Jul 27, 2016

bbondy Jul 27, 2016

Choose a reason for hiding this comment

bbondy Jul 27, 2016

Choose a reason for hiding this comment

bbondy commented Jul 27, 2016

bbondy commented Jul 29, 2016

bbondy commented Jul 29, 2016

darkdh commented Jul 29, 2016

bbondy commented Jul 29, 2016

bbondy Jul 29, 2016

Choose a reason for hiding this comment

bradleyrichter commented Jul 29, 2016

bbondy Jul 29, 2016

Choose a reason for hiding this comment

bbondy commented Jul 29, 2016

darkdh commented Jul 29, 2016

luixxiul commented Jul 29, 2016

bradleyrichter commented Jul 29, 2016

bradleyrichter commented Jul 29, 2016

luixxiul commented Jul 29, 2016

darkdh commented Jul 30, 2016

bbondy commented Jul 31, 2016

diracdeltas commented Aug 1, 2016

darkdh commented Aug 2, 2016

bradleyrichter commented Aug 2, 2016

darkdh commented Aug 2, 2016

darkdh commented Aug 2, 2016

bradleyrichter commented Aug 2, 2016

bbondy commented Aug 2, 2016

darkdh commented Jul 26, 2016 •

edited

Loading

darkdh commented Jul 27, 2016 •

edited

Loading