-
Notifications
You must be signed in to change notification settings - Fork 869
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
New Tabs Page: Enable NTP to be theme aware #3115
Conversation
3ba14b4
to
c2523fa
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.
++ 🎨 🕶 🌡
CI didn't run because it was marked draft, and some reason hasn't re-run automatically, so I'll press the button now. |
😎👌 |
Rebasing as latest Jenkins build failed due to a version mismatch |
c2523fa
to
eca2c70
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.
LGTM
triggered a new rebuild |
changing milestones as this work landed only in 0.70.x |
Closes brave/brave-browser#5014
Themes/Styling were pre-built in an earlier PR for storybook (brave/brave-ui#485)
Quick view:
Submitter Checklist:
npm run lint
)git rebase master
(if needed).git rebase -i
to squash commits (if needed).Test Plan:
1 Set theme settings to 'Light'/'Dark'/'Same as OSX' from brave://settings
2 Open NTP's customization menu
3 Menu should be themed accordingly
4. Menu should dynamically update theme when if "Same as OS theme"* is set
*Assuming native OS supports dark mode - otherwise this option wouldn't be available from settings
Reviewer Checklist:
After-merge Checklist:
changes has landed on.