-
Notifications
You must be signed in to change notification settings - Fork 896
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
Added select theme UI to the new onboarding flow #16326
Conversation
A Storybook has been deployed to preview UI for the latest push |
89e9368
to
308220c
Compare
A Storybook has been deployed to preview UI for the latest push |
c9b58b6
to
35819de
Compare
A Storybook has been deployed to preview UI for the latest push |
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.
On the design, the icon for each of the 3 theme options is vertically aligned at the same point. Obviously we have to account for different languages to have different string lengths, but it seems we could do better in this implementation to ensure they are aligned most of the time. For example, aligning the icon to the same vertical position and allowing the string to grow downwards, increasing the height of its own box (and the 2 others, via a flex parent).
browser/ui/webui/brave_welcome_ui.cc
Outdated
web_ui()->RegisterMessageCallback( | ||
"setBraveThemeType", | ||
base::BindRepeating(&WelcomeDOMHandler::SetBraveThemeType, | ||
base::Unretained(this))); |
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.
I don't think you need to add a new message type since this page already has access to the chrome.braveTheme.setBraveThemeType('Light' | 'Dark' | '')
API
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.
done
A Storybook has been deployed to preview UI for the latest push |
f64010c
to
8262f16
Compare
A Storybook has been deployed to preview UI for the latest push |
8262f16
to
ffa42de
Compare
A Storybook has been deployed to preview UI for the latest push |
ffa42de
to
195ba7b
Compare
A Storybook has been deployed to preview UI for the latest push |
195ba7b
to
5e49055
Compare
A Storybook has been deployed to preview UI for the latest push |
5e49055
to
09cab4f
Compare
A Storybook has been deployed to preview UI for the latest push |
Resolves brave/brave-browser#27170
This PR adds a new UI screen when there are no browser profiles. The screen allows a user to select a browser theme.
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
wikinpm run lint
,npm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan: