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

about:newtabs - have relevant results shown (implement frecency) #5338

Closed
bsclifton opened this issue Nov 2, 2016 · 16 comments
Closed

about:newtabs - have relevant results shown (implement frecency) #5338

bsclifton opened this issue Nov 2, 2016 · 16 comments
Labels
design A design change, especially one which needs input from the design team. feature/newtab fixed-with-brave-core This issue will automatically resolved with the replacement of Muon with Brave Core. wontfix

Comments

@bsclifton
Copy link
Member

cc: @bbondy @bradleyrichter

With some favorite sites up in the "Top Sites" area, the items are not easy to identify.
screen shot 2016-11-01 at 5 19 07 pm

We need to find a better way to represent bookmarks which have the same favicon:

  • having a bunch of medium.com posts pinned or in favs, which will all have the same icon
  • bookmarking multiple parts of brianbondy.com 😄
  • tweets
@bsclifton bsclifton added design A design change, especially one which needs input from the design team. feature/newtab labels Nov 2, 2016
@bsclifton bsclifton added this to the 0.12.9dev milestone Nov 2, 2016
@luixxiul
Copy link
Contributor

luixxiul commented Nov 2, 2016

Eventually you will have this one..

screenshot 2016-11-02 11 41 43

Is this a card game? :-D

@bsclifton
Copy link
Member Author

@srirambv brought up a great concern here (similar to @luixxiul's concern) #5347

Multiple sites on the same origin will each take up a tile. Is this behavior that we want?

@bradleyrichter
Copy link
Contributor

We need to dedupe. The example above would simply make GitHub the top site and you would have a link to github.com on a single tile.

On Nov 2, 2016, at 11:11 AM, Brian Clifton notifications@github.com wrote:

@srirambv brought up a great concern here (similar to @luixxiul's concern) #5347

Multiple sites on the same origin will each take up a tile. Is this behavior that we want?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@bsclifton
Copy link
Member Author

bsclifton commented Nov 3, 2016

Requirements (please edit this post as needed):

Brad's additions:

  • let's make sure we are using the same text color code as the tabs which should prevent low or zero contrast results
  • set opacity of mouse-hover popup tools to 80% so they are always visible
  • make sure default view is 1 row, and cycles as 1-2-3-1-2-3

@bsclifton
Copy link
Member Author

Also- if we wanted nicer high-res non-favicons, we could consider spriting up like a top 20 sheet

@bsclifton bsclifton modified the milestones: 0.12.10 release , 0.12.9dev Nov 8, 2016
@bsclifton
Copy link
Member Author

Moving to 0.12.10. For 0.12.9, the ideal behavior would be to limit folks to only 1 row of icons (until we're able to resolve this issue and the other issues)

cc: @cezaraugusto

@cndouglas
Copy link

Related issue: #5482 (only one row of topSites)

@bsclifton bsclifton self-assigned this Nov 9, 2016
@bsclifton bsclifton changed the title about:newtabs - site icons are not easy to identify about:newtabs - only show one site per domain Nov 9, 2016
@bbondy
Copy link
Member

bbondy commented Nov 11, 2016

Moving to 0.12.10 but would consider moving back if a PR is in place.

@bbondy bbondy modified the milestones: 0.12.10 release , 0.12.9dev Nov 11, 2016
@bsclifton bsclifton changed the title about:newtabs - only show one site per domain about:newtabs - have relevant results shown Nov 14, 2016
@bbondy
Copy link
Member

bbondy commented Nov 14, 2016

Please update to use the same algorithm with frecency as is done for urlbar suggestions.

@bsclifton
Copy link
Member Author

Note per our convo: we can use the frecency algorithm from suggestions.js, but we'd need to move the code to the renderer process

@bsclifton bsclifton changed the title about:newtabs - have relevant results shown about:newtabs - have relevant results shown (implement frecency) Nov 14, 2016
@bradleyrichter
Copy link
Contributor

@bsclifton Do we need a separate issue for deduping as a first pass or combine it with frecency?

@bsclifton
Copy link
Member Author

@bradleyrichter deduping should be done now (the first pass that is). If you choose to pin a site, then it won't count as a dupe (ex: you'll be shown 1 other result from that domain).

You will notice that the duping is based on hostname (which does not remove the subdomain name), which means https://blog.brave.com/ and https://brave.com/ count as two sites.. We could change this logic and instead strip off subdomain?

@bradleyrichter
Copy link
Contributor

google has so many sites/tools with subs so let's go with that. (each sub counts as a sep site)

@bsclifton bsclifton modified the milestones: 0.12.11, 0.12.10 release Nov 17, 2016
@bsclifton
Copy link
Member Author

With deadline for milestone being tonite, moving to 0.12.11

@bbondy bbondy modified the milestones: Backlog, 0.12.11 Nov 18, 2016
@bbondy
Copy link
Member

bbondy commented Nov 18, 2016

This would be great to get to use Aubrey's frecency, but since we only show 6 tiles currently and they are pretty accurate most of the time as is we can move to backlog for now.

The fix here is to use this sorting:
https://github.com/brave/browser-laptop/blob/master/app/renderer/lib/suggestion.js

But I think suggestion.js has to move to common.

@bsclifton bsclifton removed their assignment Nov 28, 2016
@bbondy bbondy removed this from the Backlog milestone Oct 19, 2017
@bsclifton bsclifton added this to the Triage Backlog milestone Nov 27, 2017
@bsclifton bsclifton added wontfix fixed-with-brave-core This issue will automatically resolved with the replacement of Muon with Brave Core. labels Sep 3, 2018
@bsclifton bsclifton removed this from the Triage Backlog milestone Sep 3, 2018
@bsclifton
Copy link
Member Author

This should be completely fixed with brave-core

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
design A design change, especially one which needs input from the design team. feature/newtab fixed-with-brave-core This issue will automatically resolved with the replacement of Muon with Brave Core. wontfix
Projects
None yet
Development

No branches or pull requests

5 participants