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 Keep in dock option to first run dialog and settings #14714

Merged
merged 2 commits into from
Oct 28, 2022

Conversation

simonhong
Copy link
Member

@simonhong simonhong commented Aug 19, 2022

fix brave/brave-browser#24055

First run dialog:
Screen Shot 2022-10-28 at 8 54 25 AM

Settings: not added to dock
Screen Shot 2022-10-28 at 8 52 18 AM

After added to dock
Screen Shot 2022-10-28 at 8 52 26 AM

Resolves

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

See the linked issue

@simonhong simonhong self-assigned this Aug 19, 2022
@simonhong simonhong force-pushed the add_to_dock_when_brave_is_default branch from c0c7654 to 4e9631b Compare August 19, 2022 07:13
@simonhong simonhong changed the title WIP: Keep in dock when Brave is set as a default browser Keep in dock when Brave is set as a default browser Aug 19, 2022
@simonhong simonhong marked this pull request as ready for review August 19, 2022 07:42
@simonhong simonhong requested a review from a team as a code owner August 19, 2022 07:42
@simonhong simonhong requested review from sangwoo108 and goodov August 19, 2022 07:42
@simonhong simonhong force-pushed the add_to_dock_when_brave_is_default branch from 4e9631b to 890c574 Compare August 19, 2022 07:42
Copy link
Contributor

@sangwoo108 sangwoo108 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a suggestion

browser/brave_shell_integration.h Show resolved Hide resolved
@simonhong simonhong marked this pull request as draft August 23, 2022 08:48
@simonhong simonhong force-pushed the add_to_dock_when_brave_is_default branch from 9fc8731 to fb49a9e Compare August 23, 2022 10:27
@simonhong simonhong changed the title Keep in dock when Brave is set as a default browser WIP: Keep in dock when Brave is set as a default browser Aug 23, 2022
@simonhong
Copy link
Member Author

simonhong commented Aug 23, 2022

Rebased and set as draft as we'll add asking to users to pin(dock).

@simonhong simonhong marked this pull request as ready for review October 4, 2022 06:16
@simonhong simonhong marked this pull request as draft October 4, 2022 06:16
@simonhong simonhong changed the title WIP: Keep in dock when Brave is set as a default browser Add Keep in dock option to first run dialog and settings Oct 26, 2022
@simonhong simonhong marked this pull request as ready for review October 26, 2022 06:19
@simonhong simonhong force-pushed the add_to_dock_when_brave_is_default branch from fb49a9e to 836dfc7 Compare October 26, 2022 06:20
@simonhong
Copy link
Member Author

@sangwoo108 It's ready to review. PTAL again. Issue and descriptions are updated.

@simonhong simonhong requested a review from sangwoo108 October 26, 2022 06:36
Copy link
Contributor

@sangwoo108 sangwoo108 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for delayed response. LGTM

browser/brave_shell_integration_mac.h Outdated Show resolved Hide resolved
browser/brave_shell_integration_mac.mm Outdated Show resolved Hide resolved
@simonhong simonhong force-pushed the add_to_dock_when_brave_is_default branch from 836dfc7 to a26ba83 Compare October 26, 2022 06:48
@simonhong
Copy link
Member Author

simonhong commented Oct 26, 2022

@rmcfadden3 @rebron Can you check above setting option's sentence?

browser/ui/webui/settings/pin_shortcut_handler.cc Outdated Show resolved Hide resolved
app/brave_generated_resources.grd Outdated Show resolved Hide resolved
browser/brave_shell_integration.cc Outdated Show resolved Hide resolved
browser/ui/BUILD.gn Outdated Show resolved Hide resolved
browser/ui/views/brave_first_run_dialog.h Outdated Show resolved Hide resolved
@simonhong simonhong requested a review from rmcfadden3 October 27, 2022 01:01
Copy link
Member

@goodov goodov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

browser/brave_shell_integration.cc Outdated Show resolved Hide resolved
With this, we can support pin shortcut feature on other platforms.
@simonhong simonhong force-pushed the add_to_dock_when_brave_is_default branch from 603910c to 67f4be0 Compare October 27, 2022 07:33
fix brave/brave-browser#24055

User can add to dock via first run or settings.
@simonhong simonhong force-pushed the add_to_dock_when_brave_is_default branch from 67f4be0 to 3acb0aa Compare October 27, 2022 23:56
@simonhong simonhong enabled auto-merge October 27, 2022 23:57
@simonhong simonhong merged commit 685f1a3 into master Oct 28, 2022
@simonhong simonhong deleted the add_to_dock_when_brave_is_default branch October 28, 2022 03:49
@github-actions github-actions bot added this to the 1.47.x - Nightly milestone Oct 28, 2022
@stephendonner
Copy link
Contributor

stephendonner commented Nov 3, 2022

Verification PASSED using

Brave 1.47.42 Chromium: 107.0.5304.91 (Official Build) nightly (x86_64)
Revision 3d5948960d62418160796d5831a4d2d7d6c90fa8-refs/branch-heads/5304@{#1097}
OS macOS Version 11.7.1 (Build 20G918)

Prerequisite: make a non-Brave browser the system default via Apple menu -> System Preferences... -> General -> Default web browser

Screen Shot 2022-11-03 at 1 52 02 PM

Case 1: Pin Brave to dock via the Pin to Dock checkbox in the first-run dialog - PASSED

  1. installed 1.47.42
  2. launched Brave
  3. checked the Keep in Dock checkbox
  4. clicked on the Set Brave as default button
  5. confirmed I got the Do you want to change your default web browser to "Brave" or keep using "Firefox"? dialog
  6. clicked on Use "Brave"
  7. confirmed Brave is kept in the dock
  8. closed Brave
  9. relaunched by clicking on the Brave icon in the dock
  10. opened brave://settings/getStarted
  11. confirmed Brave is your default browser text
  12. confirmed Brave is already in Dock text

Confirmed Brave is successfully kept in the dock

step 3 step 5 step 7 steps 11-12
Screen Shot 2022-11-03 at 1 50 26 PM Screen Shot 2022-11-03 at 1 45 00 PM Screen Shot 2022-11-03 at 1 56 07 PM Screen Shot 2022-11-03 at 1 57 17 PM

Case 2: Unpin Brave from the dock and Pin to Dock it from brave://settings/getStarted - PASSED

  1. continued from Case 1, with Brave kept in the dock
  2. context-clicked on Brave and chose Options -> Keep in Dock
  3. reloaded brave://settings/getStarted
  4. confirmed the Dock button beside Keep in Dock is clickable
  5. clicked Dock
  6. confirmed I now saw Brave is already in Dock
  7. context-click on Brave

Confirmed Brave is docked and shows Options -> Keep in Dock enabled

step 4 step 6 step 7
Screen Shot 2022-11-03 at 2 05 34 PM Screen Shot 2022-11-03 at 1 57 17 PM Screen Shot 2022-11-03 at 2 09 29 PM

Case 3: With Pin to Dock unchecked (default), click Set Brave as default on the first-run dialog - PASSED

  1. installed 1.47.42
  2. launched Brave
  3. left Keep in Dock unchecked (default)
  4. click Set Brave as default button
  5. clicked Use "Brave" on the next dialog
  6. clicked Skip welcome tour
  7. opened brave://settings/getStarted
  8. confirmed Brave is your default browser text is shown
  9. confirmed Dock button is shown in next to Keep in Dock
  10. clicked on the Dock button
  11. confirmed the Dock button on brave://settings/getStarted disappears
  12. context-click on Brave

Confirmed Brave is docked and shows Options -> Keep in Dock enabled

step 4 step 5 step 8-9 step 11 step 12
Screen Shot 2022-11-03 at 2 15 29 PM Screen Shot 2022-11-03 at 2 16 31 PM Screen Shot 2022-11-03 at 2 05 34 PM Screen Shot 2022-11-03 at 1 57 17 PM Screen Shot 2022-11-03 at 2 09 29 PM

Case 4: Pin to Dock Brave via brave://settings/getStarted when a non-Brave browser is set as default via Apple menu' -> System Preferences -> General Default web browser - PASSED

  1. installed 1.47.42
  2. launched Brave
  3. clicked Maybe later
  4. clicked Skip welcome tour
  5. closed Brave
  6. relaunched Brave
  7. clicked on Set as default in the infobar
  8. clicked on Use "Brave"
  9. opened brave://settings/getStarted
  10. confirmed it read Brave is your default browser
  11. confirmed the Dock button is enabled
  12. clicked on the Dock button
  13. confirmed the Dock button disappeared
  14. confirmed it read Brave is already in Dock
  15. context-clicked Brave

Confirmed Brave is docked and shows Options -> Keep in Dock enabled

step 3 step 7 step 8 steps 10-11 steps 13-14 step 15
Screen Shot 2022-11-03 at 2 44 36 PM Screen Shot 2022-11-03 at 2 46 08 PM Screen Shot 2022-11-03 at 2 39 30 PM Screen Shot 2022-11-03 at 2 40 23 PM Screen Shot 2022-11-03 at 2 41 36 PM Screen Shot 2022-11-03 at 2 41 44 PM

Case 5: Upgrade from 1.46.x (nightly) -> 1.47.x (nightly) - PASSED

  1. installed 1.46.46
  2. launched Brave
  3. clicked Set Brave as default on the first-run dialog
  4. clicked Use "Brave"
  5. opened brave://settings/getStarted and confirmed it shows Brave is your default browser text
  6. upgraded to 1.47.42
  7. opened brave://settings/getStarted
  8. clicked the Dock button
  9. confirmed the Dock button disappeared, and the text now reads "Brave is already in Dock`
  10. context-clicked Brave

Confirmed Brave is docked and shows Options -> Keep in Dock enabled

step 3 step 4 step 5 step 8 step 9 step 10
Screen Shot 2022-11-03 at 3 09 11 PM Screen Shot 2022-11-03 at 3 09 15 PM Screen Shot 2022-11-03 at 3 05 34 PM Screen Shot 2022-11-03 at 3 12 05 PM Screen Shot 2022-11-03 at 3 13 43 PM Screen Shot 2022-11-03 at 3 13 54 PM

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

Successfully merging this pull request may close these issues.

Add Keep in dock option to first run dialog and settings
5 participants