-
Notifications
You must be signed in to change notification settings - Fork 887
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]: Update Discover section to match new design spec. #15755
Conversation
Begin work on scroll buttons Wire up scroll buttons Only show scroll buttons on hover Clean up discover page Popular sources should use carousel Button opacity transitions Work when the container shrinks Begin work on pages Linking Improve perf Get link to align nicely Popular page Page headers localize Keep settings open on back pressed
95e43fc
to
1756ce5
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.
The Suggestions sections is showing when there are no suggested items. Perhaps Carousel
can just return null
if there are no publisher Ids...
Good call @petemill! I've updated it. |
line-height: 18px; | ||
color: var(--interactive5); | ||
cursor: pointer; | ||
|
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.
nit: empty line
flex: 5; | ||
text-align: center; | ||
` | ||
|
||
const Spacer = styled.div`flex: 1;` |
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 an interesting way to center the text. It breaks down if the word "Back" is extremely long, but that's very unlikely!
Verified
|
Brave | 1.47.48 Chromium: 107.0.5304.91 (Official Build) nightly (x86_64) |
---|---|
Revision | 3d5948960d62418160796d5831a4d2d7d6c90fa8-refs/branch-heads/5304@{#1097} |
OS | macOS Version 13.1 (Build 22C5033e) |
Steps:
- installed
1.47.48
- launched Brave
- clicked on
Skip welcome tour
- opened
brave://flags
- set
brave://flags/#brave-news-v2
toEnabled
- clicked
Relaunch
- opened a new-tab page
- clicked on
Customize
- clicked on
Brave News
- clicked on
Turn on Brave News
- checked the
Customize Brave News
dialog
Confirmed the following changes:
Popular
sources appeared firstSuggestions
appeared second (after following a few sources, then returning to this dialog)Channels
appeared third- All channels were visible (no
Show more
button) Popular
&Suggestions
appeared in a carousel, with aView All
link taking me to a full page view- removed previous
Sources
section
order of appearance |
all channels visible |
carousel |
---|---|---|
[Brave News]: Update Discover section to match new design spec.
Resolves brave/brave-browser#26462
Screencast.from.2022-11-02.17-19-05.webm
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: