-
Notifications
You must be signed in to change notification settings - Fork 920
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Implement "Desktop Welcome Pages" feature (v2) #26154
Conversation
0bc75b7
to
0a4fc38
Compare
A Storybook has been deployed to preview UI for the latest push |
c47d85a
to
01aa909
Compare
01aa909
to
4e7929a
Compare
4e7929a
to
6b75202
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.
Looks really good @zenparsing. I left a few comments, but this is really well done.
browser/ui/webui/brave_education/browser_education_page_delegate.cc
Outdated
Show resolved
Hide resolved
browser/ui/webui/brave_education/browser_education_page_delegate.cc
Outdated
Show resolved
Hide resolved
browser/ui/webui/brave_education/education_page_handler_unitttest.cc
Outdated
Show resolved
Hide resolved
6e63b2b
to
1350631
Compare
- brave_education is based on whats_new - brave_browser_commands is based on browser_commands Fixes brave/brave-browser#42562 Uses `use_brave_grit` / `brave_or_default_grit` as a work-around to avoid patching the resources IDs in `tools/gritsettings/resource_ids.spec`.
3515b7a
to
b21f6fa
Compare
|
b21f6fa
to
9354414
Compare
Includes presubmit fixes and Chromium 133 rebase fixes
9354414
to
4e3cb12
Compare
4e3cb12
to
f0539da
Compare
browser/ui/webui/brave_education/brave_education_page_handler_unitttest.cc
Outdated
Show resolved
Hide resolved
QA test plan should include what happens if the page is not available (for instance internet disabled) |
Fixed misspellings
[puLL-Merge] - brave/brave-core@26154 DescriptionThis PR adds support for a new WebUI-based education/onboarding page in Brave Browser. The education page can display content from Brave's education server (browser-education.brave.com) and allow users to trigger various browser actions like opening the wallet, rewards panel, VPN panel, or AI chat. Possible Issues
Security Hotspots
ChangesChanges
sequenceDiagram
participant WelcomeUI
participant BrowserProxy
participant WebUI
participant Server
participant Browser
WelcomeUI->>BrowserProxy: getWelcomeCompleteURL()
BrowserProxy->>WebUI: Check if education enabled
WebUI->>Server: Health check request
Server-->>WebUI: Response
WebUI-->>BrowserProxy: Return education URL if healthy
BrowserProxy-->>WelcomeUI: URL response
WelcomeUI->>Browser: Navigate to education page
Browser->>Server: Load education content
Server-->>Browser: Education content
Note over Browser: User interacts with education content
Browser->>Browser: Handle education commands
|
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.
All of my comments have been addressed and so have @diracdeltas
Also verified that matomo isn't accessible and has been removed site side for this subdomain.
This checks off all the things for privacy and security reviews so approving
There are a few TODOs before merging:
|
Fixes Linux unit test
Circling back - no update needed for test plan as this is loaded by the customer AFTER existing welcome experience happens 😄 |
Released in v1.77.2 |
Verification PASSED on
|
Resolves brave/brave-browser#40157
Resolves brave/brave-browser#42562
(for Brave employees)
Privacy / Security review
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 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:
brave://getting-started
#brave-show-getting-started-page
and restart the browser.brave://welcome
brave://getting-started
is loaded correctly.