Skip to content
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

add default search engine functionality to onboarding process #1548

Closed
rossmoody opened this issue Oct 12, 2018 · 11 comments · Fixed by brave/brave-core#2479
Closed

add default search engine functionality to onboarding process #1548

rossmoody opened this issue Oct 12, 2018 · 11 comments · Fixed by brave/brave-core#2479

Comments

@rossmoody
Copy link
Contributor

A new part of the onboarding process is the ability to set a default search engine. This requires a little conditional prep for what we present in what regions. This is likely already taken care of in Chromium code for settings but we need to link it up to this dropdown.

image

@rossmoody rossmoody added this to the 0.56.x milestone Oct 12, 2018
@rebron
Copy link
Collaborator

rebron commented Oct 12, 2018

cc: @bsclifton and @mkarolin

@lukemulks
Copy link

@bbondy bbondy modified the milestones: 1.0, 1.x Backlog Oct 30, 2018
@rebron rebron removed this from the 1.x Backlog milestone Nov 2, 2018
@bbondy bbondy added this to the 1.x Backlog milestone Nov 5, 2018
@rebron rebron added the priority/P4 Planned work. We expect to get to it "soon". label Jan 15, 2019
@rebron rebron modified the milestone: 1.x Backlog Feb 7, 2019
@rebron rebron changed the title Add default search engine functionality in onboarding add default search engine functionality to welcome flow Mar 13, 2019
@rebron rebron changed the title add default search engine functionality to welcome flow add default search engine functionality to onboarding process Mar 13, 2019
@rebron rebron added priority/P2 A bad problem. We might uplift this to the next planned release. l10n feature/search and removed priority/P4 Planned work. We expect to get to it "soon". labels May 6, 2019
@srirambv
Copy link
Contributor

srirambv commented Jul 30, 2019

Verification PASSED on macOS 10.14.5 x64 using the following build:

Brave 0.68.113 Chromium: 76.0.3809.62 (Official Build) beta (64-bit)
Revision 7b77856b3aa34d72f246d12340fc1ded8b2c0e83-refs/branch-heads/3809@{#798}
OS Mac OS X
  • ensured that Google is always selected as the default
  • ensured that once you've selected a search engine from the drop down, it's changed via brave://settings/search instantly
  • ensured that your search engine selection is still selected if you close onboarding once you've selected the search engine
  • ensured that your search engine selection is still selected when going through the entire onboarding process
  • ensured that DDG, Google, Bing, Startpage Qwant are the only available options
  • ensured the URL uses the correct search engine once it has been selected
  • ensured that the brave://welcome drop down always displays Default next to the search ending that's selected

Created #5453.

Verification passed on

Brave 0.68.113 Chromium: 76.0.3809.62 (Official Build) beta (64-bit)
Revision 7b77856b3aa34d72f246d12340fc1ded8b2c0e83-refs/branch-heads/3809@{#798}
OS Linux
  • Verified able to choose any of the available SE from the dropdown list
  • Verified selected SE is set as default in settings
    image

@rossmoody @rebron @imptrx could we eliminate the Set Default button and just use the dropdrown to trigger the set default button action? This reduces one more click that you have to go through in the onboarding screen. Whichever option is selected should automatically set it in settings

Verification passed on

Brave 0.68.119 Chromium: 76.0.3809.87 (Official Build) beta (64-bit)
Revision 111fe1e15d5ced26080a7dc239bcfe70f6c49aad-refs/branch-heads/3809@{#967}
OS Windows 10 OS Version 1803 (Build 17134.523)
  • ensured that Google is always selected as the default
  • ensured that once you've selected a search engine from the drop-down, it's changed via brave://settings/search instantly
  • ensured that your search engine selection is still selected if you close onboarding once you've selected the search engine
  • ensured that your search engine selection is still selected when going through the entire onboarding process
  • ensured that DDG, Google, Bing, Startpage Qwant are the only available options
  • ensured the URL uses the correct search engine once it has been selected
  • ensured that the brave://welcome drop-down always displays Default next to the search ending that's selected

@kjozwiak
Copy link
Member

@rossmoody @rebron @imptrx could we eliminate the Set Default button and just use the dropdrown to trigger the set default button action? This reduces one more click that you have to go through in the onboarding screen. Whichever option is selected should automatically set it in settings

I would suggest the same thing with the "Theme" section under 0.70.18 Chromium: 76.0.3809.72 (Official Build) nightly. Changing between the theming option automatically applies the change without the need to click on Confirm. Example:

Screen Shot 2019-07-30 at 12 00 36 PM

@rossmoody @rebron @imptrx thoughts? Should we open a new issue so we can discuss possibly removing the Set default & Confirm buttons?

@rebron
Copy link
Collaborator

rebron commented Jul 30, 2019

We want to keep those buttons since set default and confirm also act as next buttons. Otherwise, selecting just from the drop down and then moving users to the next screen would feel too abrupt an action, especially so on theme choice for example. @rossmoody to confirm though.

@rossmoody
Copy link
Contributor Author

That’s the idea but @petemill brought up a similar concern and under the hood I believe the setting is set when the drop down is changed not on button click so though I don’t think we can assume advancing on first selection is right maybe the “next” button to could serve as the confirm

@srirambv
Copy link
Contributor

srirambv commented Jul 30, 2019

We want to keep those buttons since set default and confirm also act as next buttons.

Seems redundant as there is the next button as well
image

We can just text of Next button to Set Default so that the click sets and moves forward in the flow.

@kjozwiak
Copy link
Member

yup, as @srirambv & @rossmoody mentioned, switching the setting via the drop down boxes changes the setting and doesn't move the user to the next screen unless they click on Next -> or one of the bubbles that indicate where you are in the onboarding process.

Both Set default for search engines & Confirm for themes are basically second Next buttons.

@srirambv
Copy link
Contributor

Also confirm theme seems to open up setting page. Since its a drop-down should probably have the same behaviour as SE and not really switch to the settings page. That can probably be discussed on a different issue

@kjozwiak
Copy link
Member

Also confirm theme seems to open up setting page. Since its a drop-down should probably have the same behaviour as SE and not really switch to the settings page. That can probably be discussed on a different issue

That's being addressed in 0.70.x via #4992. Already merged. Double checked with 0.70.18 CR: 76.0.3809.72 and it works as expected 👍

@rossmoody
Copy link
Contributor Author

Well, there was thought for this that I think should be considered which is the user can’t assume that their search engine changed without a confirm button. So I’d vote for that to stay. “Next“ is to advance the screen in cases where a user doesn’t want to make a decision. In the case of themes the user can see their theme change in real time so that’s the confirmation. I personally still think giving the user a place to feel like the decision is confirmed is strong but in the case of themes it could be adjusted. Future ui had the next button removed as well so I think we should be mindful of changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants