Add different configuration variants for Browsing Menu Sheet #2691

dus7 · 2025-11-28T16:42:34Z

Task/Issue URL: https://app.asana.com/1/137249556945/project/1206226850447395/task/1212001840997427?focus=true
Tech Design URL:
CC:

Description

Extract common menu entries creation to private common functions to prevent duplication.
Add builder methods to menu builder protocol.
Add variants for Sheet Browsing Menu, using current production methods via the builder protocol.

Testing Steps

With Sheet Menu disabled (bottom of Appearance settings or internal flag off), check browsing menu functions properly (executes action), especially for modified items:
1. Report broken site
2. Forget all
3. Settings
4. Refresh
5. Request Desktop site
Enable Sheet Browsing Menu, switch between variants. Verify setting is persisted

Impact and Risks

Medium: Could disrupt specific features or user flows

What could go wrong?

Modified menus does not function properly. This was tested multiple times and code was extracted to functions with least modifications possible.

Quality Considerations

Notes to Reviewer

Internal references:

Definition of Done | Engineering Expectations | Tech Design Template

Note

Adds new Browsing Menu Sheet configurations (B/C/D), introduces a floating footer toolbar, and refactors menu entry construction to support variant-specific builders.

Browsing Menu Sheet
- Variants: Extend BrowsingMenuClusteringVariant with b/c/d and descriptions.
- Builders: Add BrowsingMenuVariantBBuilder, BrowsingMenuVariantCBuilder, BrowsingMenuVariantDBuilder implementing custom header/section/footer compositions.
- Capability: Wire variants into BrowsingMenuSheetCapability and factory; expose variant selection/storage.
UI (SwiftUI)
- Sheet View: Add floatingToolbar footer (safe area inset) rendering model.footerItems; initialize highlightTag via State; default actionToPerform closure.
TabViewController
- Variant Routing: Choose builder via makeVariantBuilder and buildSheetBrowsingMenu.
- Refactor: Extract reusable entry maker methods (e.g., share, reload, report broken site, desktop site, settings with small/large icons) and expose them via BrowsingMenuEntryBuilding to support builders.
Project
- Add new Swift files to project and sources.

^{Written by Cursor Bugbot for commit ca4f5d5. This will update automatically on new commits. Configure here.}

github-actions · 2025-11-28T21:16:12Z

	Warnings
⚠️	PR has 773 lines of added code (excluding Xcode projects and assets). Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS against ca4f5d5

Copilot

Pull request overview

This PR adds configuration variants for the Browsing Menu Sheet presentation, enabling A/B testing of different menu layouts. The changes introduce a protocol-based architecture where menu building logic is extracted into reusable methods and variant-specific builders control menu structure.

Key Changes

Introduces BrowsingMenuVariantBuilder protocol with three concrete implementations (A, B, C) offering different menu organizations
Extracts menu entry building logic into separate build* methods in TabViewController that implement the BrowsingMenuEntryBuilding protocol
Adds BrowsingMenuSheetCapability to manage feature availability and variant selection, restricted to internal users and iOS 17+

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
BrowsingMenuVariantBuilder.swift	Defines protocols for variant builders and entry building; includes shared AI chat menu implementation
BrowsingMenuVariantABuilder.swift	Variant A implementation using existing production menu structure
BrowsingMenuVariantBBuilder.swift	Variant B implementation with "Easy Shortcuts" layout
BrowsingMenuVariantCBuilder.swift	Variant C implementation with "Easy Privacy Tools" layout
BrowsingMenuSheetCapability.swift	Manages feature availability, enablement, and variant persistence
BrowsingMenuSheetView.swift	Updates to support new model structure with header/footer/sections and tag-based highlighting
TabViewControllerMenuBuilderExtension.swift	Extracts inline menu building into reusable methods; implements BrowsingMenuEntryBuilding protocol
MainViewController.swift	Adds variant selection logic and separates sheet vs default menu launching
SettingsViewModel.swift	Adds browsing menu capability dependency and variant binding
SettingsState.swift	Adds sheetMenuVariant state property
SettingsAppearanceView.swift	Adds UI for variant selection in settings
ListExtension.swift	New utility extension for compact section spacing on iOS 17+
BrowsingMenuViewController.swift	Adds isSeparator convenience property to BrowsingMenuEntry
BrowsingMenuVariantBuilderTests.swift	Comprehensive test coverage for variant builders with mock implementations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

iOS/DuckDuckGo/BrowsingMenu/SheetPresentationMenu/BrowsingMenuVariantCBuilder.swift

iOS/DuckDuckGo/BrowsingMenu/SheetPresentationMenu/BrowsingMenuVariantBBuilder.swift

iOS/DuckDuckGo/TabViewControllerMenuBuilderExtension.swift

iOS/DuckDuckGo/BrowsingMenu/SheetPresentationMenu/BrowsingMenuSheetCapability.swift

iOS/DuckDuckGo/BrowsingMenu/SheetPresentationMenu/BrowsingMenuVariantBuilder.swift

iOS/DuckDuckGo/BrowsingMenu/SheetPresentationMenu/BrowsingMenuVariantABuilder.swift

iOS/DuckDuckGo/BrowsingMenu/SheetPresentationMenu/BrowsingMenuSheetView.swift

iOS/DuckDuckGo/MainViewController.swift

iOS/DuckDuckGoTests/BrowsingMenuSheet/BrowsingMenuVariantBuilderTests.swift

iOS/DuckDuckGo/BrowsingMenu/SheetPresentationMenu/BrowsingMenuVariantABuilder.swift

cursor

Bug: Incorrect @State initialization may break highlight feature

The @State properties highlightTag and actionToPerform are initialized using self.highlightTag = highlightRowWithTag instead of the proper SwiftUI pattern _highlightTag = State(initialValue: highlightRowWithTag). This is a known SwiftUI anti-pattern that can cause the initial value to be ignored because SwiftUI's State storage is initialized before the init body runs. The codebase consistently uses the underscore syntax elsewhere (e.g., DaxDialogView.swift, SettingsRootView.swift). The highlight animation for the "Add Favorite" onboarding flow may fail to appear as a result.

_{Please tell me if this was useful or not with a 👍 or 👎.}

iOS/DuckDuckGo/BrowsingMenu/SheetPresentationMenu/BrowsingMenuSheetView.swift#L43-L49

apple-browsers/iOS/DuckDuckGo/BrowsingMenu/SheetPresentationMenu/BrowsingMenuSheetView.swift

Lines 43 to 49 in a4a2dc4

    
           init(model: BrowsingMenuModel, highlightRowWithTag: BrowsingMenuModel.Entry.Tag? = nil, onDismiss: @escaping () -> Void) { 
        
               self.model = model 
        
               self.onDismiss = onDismiss 
        
               self.highlightTag = highlightRowWithTag 
        
               self.actionToPerform = { } 
        
           }

brindy

LGTM - tested the various variants and production menu and they seem to work fine.

Task/Issue URL: https://app.asana.com/1/137249556945/task/1212219459510183?focus=true Tech Design URL: CC: ### Description This PR adds a foundation for being able to produce multiple variants of Sheet browsing menu using existing entries. In order to allow this the following changes has been made: * Prepare a new protocol to use in sheet variants of the new menu. * Use `BrowsingMenuContext` in MainViewController to encapsulate MVC logic. * Create "A" variant of the menu which is a verbatim version of production. * Add an Appearance Setting and local storage for selecting variant option of the sheet menu (internal only). * Update the model used in `BrowsingMenuSheetView` and improve UI. ### Testing Steps 1. Turn off internal user flag, there should be no additional options in Appearance Settings 2. Do a smoke check around BrowsingMenu - how it looks in different "contexts": NTP, Duck.ai, browsing. 3. Do a general smoke check of Browsing menu.  ### Impact and Risks Medium: Could disrupt specific features or user flows #### What could go wrong? 1. Browsing menu does not look properly in different contexts (NTP, browsing, etc.) - this is not likely because the logic was encapsulated and used 1:1 comparing to previous state. 2. Additional options in Appearance Settings are visible for non-internal users - was checked in previous PR and this builds in top of existing state. ### Quality Considerations  ### Notes to Reviewer There will be a follow-up PR containing additional menu variants: #2691 --- ###### Internal references: [Definition of Done](https://app.asana.com/0/1202500774821704/1207634633537039/f) | [Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) | [Tech Design Template](https://app.asana.com/0/59792373528535/184709971311943)  --- > [!NOTE] > Introduces a builder-driven sheet Browsing Menu (variants A–D) with internal-only toggle and variant picker, refactors menu construction and integrates sheet presentation across contexts. > > - **Browsing Menu Sheet Capability & Variants** > - Add `BrowsingMenuSheetCapability` (internal-only, iOS ≥17) with enable/disable and persisted `BrowsingMenuClusteringVariant` (A–D). > - Define `BrowsingMenuVariantBuilder` protocol and implementations: `BrowsingMenuVariantABuilder/B/C/DBuilder`. > - Introduce `BrowsingMenuContext` and `BrowsingMenuEntryBuilding` to decouple menu entry construction. > - **SwiftUI Sheet Menu UI** > - Create `BrowsingMenuModel` and refactor `BrowsingMenuSheetView` to use header/sections/footer, optional row highlight, and floating toolbar; add `List.compactSectionSpacingIfAvailable`. > - **Integration** > - Main flow: determine context (NTP/AI tab/website) and launch either classic or new sheet menu; add favorite-row highlighting via tag. > - Pass capability into `SettingsViewModel`; instantiate capability in `MainViewController`. > - `TabViewController`: expose entry-making methods and `buildSheetBrowsingMenu` using selected variant; factor helpers (share, reload, report, etc.). > - **Settings** > - Add Appearance section controls (internal users): toggle "Sheet menu presentation" and pick "Menu variant"; wire bindings (`showMenuInSheet`, `sheetMenuVariant`). > - Extend `SettingsState` with `showMenuInSheet` and `sheetMenuVariant`; update `SettingsViewModel` init/bindings. > - **Misc** > - Minor utility: `BrowsingMenuEntry.isSeparator` extension; project adds new Swift files and build phases. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit e3ddb2f. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup>

dus7 force-pushed the mariusz/sheet-menu-variants branch from 0fde166 to 2638cec Compare November 28, 2025 16:45

dus7 marked this pull request as ready for review November 28, 2025 21:14

Copilot AI review requested due to automatic review settings November 28, 2025 21:14

Copilot started reviewing on behalf of dus7 November 28, 2025 21:15 View session

Copilot finished reviewing on behalf of dus7 November 28, 2025 21:17

Copilot AI reviewed Nov 28, 2025

View reviewed changes

cursor bot reviewed Nov 28, 2025

View reviewed changes

iOS/DuckDuckGo/MainViewController.swift Show resolved Hide resolved

dus7 force-pushed the mariusz/sheet-menu-variants branch from 74412b8 to 77a4244 Compare November 28, 2025 21:38

cursor bot reviewed Nov 28, 2025

View reviewed changes

iOS/DuckDuckGoTests/BrowsingMenuSheet/BrowsingMenuVariantBuilderTests.swift Outdated Show resolved Hide resolved

dus7 force-pushed the mariusz/sheet-menu-variants branch from f41be66 to 6509e25 Compare November 28, 2025 22:19

cursor bot reviewed Nov 28, 2025

View reviewed changes

iOS/DuckDuckGo/BrowsingMenu/SheetPresentationMenu/BrowsingMenuVariantABuilder.swift Show resolved Hide resolved

dus7 force-pushed the mariusz/sheet-menu-variants branch from 9203c71 to a4a2dc4 Compare November 28, 2025 23:23

dus7 changed the base branch from main to mariusz/streamline-menu-creation November 28, 2025 23:23

dus7 force-pushed the mariusz/streamline-menu-creation branch from c37c7e1 to 70ffee2 Compare November 28, 2025 23:24

cursor bot reviewed Nov 28, 2025

View reviewed changes

dus7 added 5 commits November 29, 2025 00:35

Add floating toolbar capability to Sheet menu

251531e

Add functions to build menu entries

b3ff9b6

Add variant B

fa65049

Add variant C

a830dca

Add variant D

075984d

dus7 force-pushed the mariusz/streamline-menu-creation branch from 70ffee2 to 54253a2 Compare November 28, 2025 23:36

dus7 force-pushed the mariusz/sheet-menu-variants branch from a4a2dc4 to 075984d Compare November 28, 2025 23:37

Fix state initialization

ca4f5d5

dus7 mentioned this pull request Dec 1, 2025

Use builder to create SheetBrowsingMenu #2698

Merged

dus7 requested a review from brindy December 1, 2025 07:33

brindy approved these changes Dec 1, 2025

View reviewed changes

dus7 merged commit e3ddb2f into mariusz/streamline-menu-creation Dec 1, 2025
33 checks passed

dus7 deleted the mariusz/sheet-menu-variants branch December 1, 2025 10:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add different configuration variants for Browsing Menu Sheet #2691

Add different configuration variants for Browsing Menu Sheet #2691

Uh oh!

dus7 commented Nov 28, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Nov 28, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor bot left a comment

Uh oh!

brindy left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


	init(model: BrowsingMenuModel, highlightRowWithTag: BrowsingMenuModel.Entry.Tag? = nil, onDismiss: @escaping () -> Void) {
	self.model = model
	self.onDismiss = onDismiss
	self.highlightTag = highlightRowWithTag
	self.actionToPerform = { }
	}

Add different configuration variants for Browsing Menu Sheet #2691

Add different configuration variants for Browsing Menu Sheet #2691

Uh oh!

Conversation

dus7 commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing Steps

Impact and Risks

What could go wrong?

Quality Considerations

Notes to Reviewer

Internal references:

Uh oh!

github-actions bot commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Bug: Incorrect @State initialization may break highlight feature

Uh oh!

brindy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dus7 commented Nov 28, 2025 •

edited

Loading

github-actions bot commented Nov 28, 2025 •

edited

Loading