-
Notifications
You must be signed in to change notification settings - Fork 883
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
feat(ntp): store last NTP search engine as a pref #26137
Conversation
A Storybook has been deployed to preview UI for the latest push |
61ad76b
to
d2e4072
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++ 🚀
components/brave_new_tab_ui/containers/newTab/settings/search.tsx
Outdated
Show resolved
Hide resolved
components/brave_new_tab_ui/components/search/SearchResults.tsx
Outdated
Show resolved
Hide resolved
components/brave_new_tab_ui/components/search/SearchPlaceholder.tsx
Outdated
Show resolved
Hide resolved
components/brave_new_tab_ui/components/search/SearchContext.tsx
Outdated
Show resolved
Hide resolved
8f63351
to
549e891
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! Thanks @IanKrieger for making those changes, looks much neater now!
components/brave_new_tab_ui/containers/newTab/settings/search.tsx
Outdated
Show resolved
Hide resolved
components/brave_new_tab_ui/components/search/EngineContext.tsx
Outdated
Show resolved
Hide resolved
549e891
to
860cf0f
Compare
Released in v1.73.51 |
Resolves brave/brave-browser#41789
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:
Currently, the search engine that is the default for the search-widget on the New Tab Page (NTP), is managed by
localStorage
. For more information on whatlocalStorage
is, please see documentation on the Web Storage API. This feature intends on replacing the value which is stored as a property of the window, and instead have it be a preference of the browser. This way, there is no way that it can be unintentionally lost.Prerequisite
If you do not see the NTP search widget, on the NTP go to the
Customize
button on the bottom right, and got to theSearch
settings, flip the switch on.Enabling different engines
In the
Search
section of the settings, check / uncheck engines as seen fit.Changing NTP widget default
customize list
to use moreExisting Profile Considerations
Single Search Engine Selected
Scenario 1
Scenario 2
Multiple Search Engine Selected
Scenario 1
Scenario 2