Implement static prompts for add to dock #3790

jotaemepereira · 2025-01-27T17:58:51Z

Task/Issue URL: https://app.asana.com/0/1204006570077678/1209173259474635/f
Tech Design URL:
CC:

Description

This adds new static entry points to add to the dock. Now, the adding to dock setting can be used from the main menu, and more options menu, and also in the Settings -> Default Browser -> Shortcuts section.

In the more options menu, we will show a blue dot (both in the more options button and in the menu item), the notification dot will disappear after the user opens and closes the more options menu. If you want to check the blue dot again, I’ve added a debug menu in the Reset Data -> Reset Add To Dock more options menu notification.

Acceptance criteria

AC1 - AppStore users should not see any Add To Dock prompt
Given an AppStore user, when the user does not have DDG in the dock, then it should never see one of the prompts.

AC2 - If the user is non-AppStore, and DDG is not in the dock.
Given a non-AppStore user, when the user does not have DDG in the dock, then the user should see a 'Add to Dock' prompt on the main menu, more options, and in the Settings → Default Browser preference pane.

AC3 - If the user is non-AppStore, and DDG is in the dock.
Given a non-AppStore user, when the user does have DDG in the dock, then the user should not see the 'Add to Dock' prompt on the main menu and more options, and in Settings → Default Browser, you should see 'DuckDuckGo is in your Dock.

Definition of Done:

Does this PR satisfy our Definition of Done?

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

github-actions · 2025-01-27T18:00:36Z

	Warnings
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

	Messages
📖	You seem to be updating localized strings. Make sure that you request translations and include translated strings before you ship your change. See Localization Guidelines for more information.

Generated by 🚫 dangerJS against c32421f

ayoy

Hi @jotaemepereira! The feature works great 👏 I've left some comments in the code as I think it would be great to have it cleaned up a bit. Let me know if you have any concerns about it. Thanks!

ayoy · 2025-01-28T14:16:12Z

DuckDuckGo/Common/Localizables/UserText.swift

@@ -663,6 +663,7 @@ struct UserText {
    static let isAddedToDock = NSLocalizedString("preferences.is-added-to-dock", value: "DuckDuckGo is added to the Dock.", comment: "Indicates that the browser is added to the macOS system Dock")
    static let isNotAddedToDock = NSLocalizedString("preferences.not-added-to-dock", value: "DuckDuckGo is not added to the Dock.", comment: "Indicate that the browser is not added to macOS system Dock")
    static let addToDock = NSLocalizedString("preferences.add-to-dock", value: "Add to Dock…", comment: "Action button to add the app to the Dock")
+    static let addDuckDuckGoToDock = NSLocalizedString("preferences.add-to-dock", value: "Add DuckDuckGo To Dock…", comment: "Action button to add the app to the Dock")


Not sure if this has been reviewed already, but usually the word to in macOS menus seems to be lowercase. Just dropping a note that we may want to change this:

Good call, copy was approved, but I can bring this to their attention.

ayoy · 2025-01-30T13:36:17Z

DuckDuckGo/NavigationBar/View/MenuItemWithNotificationDot.swift

+    @State private var isHovered: Bool = false
+
+    var body: some View {
+            ZStack {


The body of this body looks misaligned :)

Good eye 😬 . Fixed!

ayoy · 2025-01-30T13:38:48Z

DuckDuckGo/NavigationBar/View/MoreOptionsMenu.swift

@@ -188,6 +216,16 @@ final class MoreOptionsMenu: NSMenu, NSMenuDelegate {
        addItem(preferencesItem)
    }

+    private func createMenuItemWithFeatureIndicator(title: String, image: NSImage, onTap: @escaping () -> Void) -> NSView {


Really great job on it. I probably wouldn't consider going with SwiftUI there :) it looks indistinguishable from a regular menu item though 💪

Thanks! I ran out of ideas of how to add the custom circle to the right, maybe something easier was out there and I missed it 😅 .

ayoy · 2025-01-30T13:48:10Z

DuckDuckGo/Application/DockCustomizer.swift

+    var wasFeatureShownPublisher: AnyPublisher<Bool, Never> {
+        $isFeatureShownFromMoreOptionsMenu.eraseToAnyPublisher()


Why are we using present and past tense in the same context? Passive voice is a bit strange in API naming, so perhaps we could call everything didShowFeatureFromMoreOptionsMenu (and didShowPublisher)?

Why? Because I’m bad at naming 😂 . Let me user your approach.

ayoy · 2025-01-30T13:56:58Z

DuckDuckGo/Menus/MainMenu.swift

@@ -438,6 +443,11 @@ final class MainMenu: NSMenu {
    override func update() {
        super.update()

+#if !APPSTORE


Sorry to be a pain, and I recall telling you about NSApp.isSandboxed earlier just to add to the mix...

I see that we're using #if !APPSTORE and #if SPARKLE in parts of the implementation. And then NSApp.isSandboxed in unit tests. Would it be possible to unify these approaches? Ultimately it's sandboxing that prevents us from adding to Dock so perhaps we could use NSApp.isSandboxed everywhere? Otherwise just rely on either SPARKLE or !APPSTORE.

You are not a pain at all, and I agree that we should be consistent.

The big difference between the two is that using a compilation flag does not include the code in the binary while using NSApp.isSandboxed adds it.
The compilation flag seems safer, given that the App Store build would not compile, while the second one is easier to mock, but could lead to a possible issue if something messes up the NSApp.isSandboxed

I’m going to play the safe approach and use the compilation flag, I’m going to use SPARKLE build because it easier to read that using the negative.

ayoy

Looks great now, thanks @jotaemepereira!

jotaemepereira requested a review from ayoy January 27, 2025 18:05

jotaemepereira added 7 commits January 29, 2025 11:45

Implement Add To Dock static prompts

1585552

Add user translations

76a438e

Add tests for main menu

dee65e6

Add tests for more options menu

7cc6b46

Remove TODO and add new circle asset

6d76bb1

Fix appstore tests

f61c964

Add circle check colors

2357cba

jotaemepereira force-pushed the juan/implement-static-prompts-for-add-to-dock branch 2 times, most recently from 830f4fb to 9ca4c67 Compare January 29, 2025 17:57

Add blue dot indicator to menu item

f64d4c1

jotaemepereira force-pushed the juan/implement-static-prompts-for-add-to-dock branch from 9ca4c67 to f64d4c1 Compare January 29, 2025 18:15

jotaemepereira added 6 commits January 29, 2025 15:46

Close menu when browser is added to dock on custom menu item

7f3796c

Fix SwiftLint

9306c79

Fix building on App Store builds

e5bd91e

Update menu item hover light color

943f28a

Fix corner radius on hover state

efde279

Fix corner radius issue on custom menu item

70c4c13

ayoy reviewed Jan 30, 2025

View reviewed changes

jotaemepereira added 6 commits January 30, 2025 14:52

Fix alignment

e8a9f10

Use sparkle compilation flag

c9fa96f

Fix tests to use correct compilation flag

8a8c545

Improve naming on dock protocol operations

92c938b

Fix swift lint

c01aa7a

Fix unit tests flags

2d8fa5d

jotaemepereira requested a review from ayoy January 30, 2025 19:42

ayoy approved these changes Jan 30, 2025

View reviewed changes

ayoy assigned jotaemepereira Jan 30, 2025

Implement blue-dot new business logic

8b04bad

Use 12 hours for timer

c32421f

jotaemepereira merged commit 8080c1f into main Jan 31, 2025
20 checks passed

jotaemepereira deleted the juan/implement-static-prompts-for-add-to-dock branch January 31, 2025 20:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement static prompts for add to dock #3790

Implement static prompts for add to dock #3790

jotaemepereira commented Jan 27, 2025 •

edited

Loading

github-actions bot commented Jan 27, 2025 •

edited

Loading

ayoy left a comment

ayoy Jan 28, 2025

jotaemepereira Jan 30, 2025

ayoy Jan 30, 2025 •

edited

Loading

jotaemepereira Jan 30, 2025

ayoy Jan 30, 2025

jotaemepereira Jan 30, 2025

ayoy Jan 30, 2025

jotaemepereira Jan 30, 2025

ayoy Jan 30, 2025

jotaemepereira Jan 30, 2025 •

edited

Loading

ayoy left a comment

		var wasFeatureShownPublisher: AnyPublisher<Bool, Never> {
		$isFeatureShownFromMoreOptionsMenu.eraseToAnyPublisher()

Implement static prompts for add to dock #3790

Implement static prompts for add to dock #3790

Conversation

jotaemepereira commented Jan 27, 2025 • edited Loading

Description

Acceptance criteria

Internal references:

github-actions bot commented Jan 27, 2025 • edited Loading

ayoy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ayoy Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jotaemepereira Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

ayoy left a comment

Choose a reason for hiding this comment

jotaemepereira commented Jan 27, 2025 •

edited

Loading

github-actions bot commented Jan 27, 2025 •

edited

Loading

ayoy Jan 30, 2025 •

edited

Loading

jotaemepereira Jan 30, 2025 •

edited

Loading