-
Notifications
You must be signed in to change notification settings - Fork 893
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Brave News Source Suggestions #15447
Conversation
1d4af76
to
fb9b2b1
Compare
5086626
to
69a0d4e
Compare
69a0d4e
to
46ba794
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.
This is working pretty nicely! Just some minor nits.
base::Unretained(this), std::move(callback))); | ||
} | ||
|
||
void SuggestionsController::GetSuggestedPublisherIdsWithHistory( |
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.
Perhaps either this function and/or GetVisitWeightings could do with some unit tests
components/brave_new_tab_ui/components/default/braveToday/customize/Context.tsx
Show resolved
Hide resolved
components/brave_new_tab_ui/components/default/braveToday/customize/Suggestions.tsx
Outdated
Show resolved
Hide resolved
components/brave_new_tab_ui/components/default/braveToday/customize/Suggestions.tsx
Outdated
Show resolved
Hide resolved
a03f34e
to
a1464d0
Compare
I ended up fixing some of these minor nits, because I had everything open - hope you don't mind! |
WIP suggestions WIP suggestions Working suggestions Add UI for viewing suggestions Fix suggestion ordering Only show suggestions when not searching Add handy comment Begin work on weightings for history
d218f38
to
eb6debc
Compare
Resolves brave/brave-browser#25563
Limitations
This PR isn't a complete implementation of suggestions, but I think it's a good first pass, and provides sensible suggestions. I'm documenting a few of the limitations here:
a) Whether a source has been visited
b) How similar a source is to subscribed sources.
c) How similar a source is to visited sources.
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
,npm run lint
,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan:
The Drive
publisherPopularMechanics
,Forbes
,Car and Driver
. Suggestions may (or may not) be slightly different