Skip to content

Conversation

@malmstein
Copy link
Contributor

@malmstein malmstein commented Dec 17, 2025

Task/Issue URL: https://app.asana.com/1/137249556945/project/488551667048375/task/1212358379355424

Description

This PR adds the initial UX for the Duck.ai contextual shit

Steps to test this PR

Enable Contextual FF - Send prompt

  • Fresh install and open AI Features
  • Enable Contextual Mode (Feature Flag Inventory)
  • Navigate to a site and press the Duck.ai button in the omnibar
  • Verify the Bottom Sheet appears
  • Write something and tap send
  • Verify prompt sent to Duck.ai

Enable Contextual FF - NTP

  • Fresh install and open AI Features
  • Enable Contextual Mode (Feature Flag Inventory)
  • Open New Tab Page and press the Duck.ai button in the omnibar
  • Verify the Bottom Sheet doesn’t appear

Disable Contextual FF

  • Fresh install and open AI Features
  • Enable Contextual Mode
  • Navigate to a site and press the Duck.ai button in the omnibar
  • Verify Duck.ai opens (not in sheet)

Note

Introduces a Duck.ai contextual bottom sheet launched from the omnibar (non‑NTP), with new command/factory, ViewModel/Fragment wiring, tests, and supporting resources guarded by a feature flag.

  • Duck.ai contextual mode (UX)
    • New bottom sheet DuckChatContextualBottomSheet with prompt input + embedded WebView, downloads, and JS messaging.
    • Factory DuckChatContextualBottomSheetFactory to create/show the sheet from BrowserTabFragment.
  • ViewModel/command flow
    • Adds Command.ShowDuckAIContextualMode and handles it in BrowserTabFragment.
    • BrowserTabViewModel.onDuckChatOmnibarButtonClicked now branches: showContextualMode (non‑NTP) → show sheet; showFullScreenMode → navigate; else → open legacy chat.
  • Feature toggles
    • contextualMode() added/updated in DuckChatFeature (default FALSE).
  • Tests
    • New tests in BrowserTabViewModelTest validating contextual vs full‑screen/NTP behavior.
  • Resources
    • New layout duckchat/.../bottom_sheet_duck_ai_contextual.xml, styles, strings, dimens.
    • New drawables (e.g., duck_ai_prompt_background.xml, ic_arrow_down_right_16.xml, ic_duck_ai_color_24.xml, ic_expand_24.xml).
  • Misc
    • Minor constant visibility tweak in DuckChatWebViewFragment (exposes REQUEST_CODE_CHOOSE_FILE).

Written by Cursor Bugbot for commit bb721d4. This will update automatically on new commits. Configure here.

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@malmstein malmstein changed the title Duck.ai: Contextual HP Duck.ai: Contextual Sheet UX Dec 17, 2025
@malmstein malmstein force-pushed the feature/david/12-10-duck.ai_contextual_hp branch 2 times, most recently from be969ee to 0352de0 Compare December 19, 2025 13:13
@malmstein malmstein force-pushed the feature/david/12-10-duck.ai_contextual_hp branch from 0352de0 to f8ee4ca Compare December 19, 2025 19:54
@malmstein malmstein marked this pull request as ready for review December 19, 2025 20:00
duckChatContextualSheet = duckChatContextualBottomSheetFactory.create()
}
duckChatContextualSheet?.show(childFragmentManager, DuckChatContextualBottomSheet.TAG)
}
Copy link

Choose a reason for hiding this comment

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

Bug: Cached DialogFragment causes crash on second show

The duckChatContextualSheet reference is cached and reused, but BottomSheetDialogFragment instances cannot be shown again after being dismissed because they go through onDestroy. When the user dismisses the bottom sheet and tries to open it a second time, calling show() on the destroyed fragment will throw an IllegalStateException and crash the app. The reference needs to be cleared when the sheet is dismissed, or a new instance needs to be created each time.

Additional Locations (1)

Fix in Cursor Fix in Web

requestFileDownload(url, contentDisposition, mimeType)
}
}
}
Copy link

Choose a reason for hiding this comment

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

Bug: Permission request called from wrong thread will crash

The download listener launches a coroutine on dispatcherProvider.io() and calls requestFileDownload(). When storage permission is not granted, this function calls requestWriteStoragePermission() which invokes requestPermissions() - a Fragment UI method that must be called on the main thread. Calling this from the IO dispatcher will cause a crash or undefined behavior when the user tries to download a file without storage permission.

Additional Locations (1)

Fix in Cursor Fix in Web

}

val url = arguments?.getString(KEY_DUCK_AI_URL) ?: "https://duckduckgo.com/?q=DuckDuckGo+AI+Chat&ia=chat&duckai=5"
binding.simpleWebview.loadUrl(url)
Copy link

Choose a reason for hiding this comment

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

Bug: WebView URL reloads unconditionally losing conversation state

In onViewCreated, the WebView URL is loaded unconditionally at lines 336-337, even when sheetMode is WEBVIEW (indicating the user was mid-conversation). When the view is recreated (e.g., configuration change or sheet dismiss/re-show), restoreWebState() is called but then the default URL is loaded anyway, resetting any ongoing chat conversation. The URL loading logic needs to be conditioned on sheetMode to avoid losing the user's chat state.

Fix in Cursor Fix in Web


private var pendingFileDownload: PendingFileDownload? = null
private val downloadMessagesJob = ConflatedJob()
private var isExpanded = false
Copy link

Choose a reason for hiding this comment

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

Bug: Keyboard insets broken after sheet is expanded once

The isExpanded flag is set to true when the sheet reaches STATE_EXPANDED but is never reset to false in onDestroyView or when the sheet is dismissed. Since the fragment instance is reused (cached in BrowserTabFragment), when the user reopens the sheet after previously expanding it, isExpanded remains true. This causes the keyboard insets logic (which checks if (!isExpanded)) to skip applying bottom padding adjustments, resulting in the keyboard overlapping the input field on subsequent opens.

Additional Locations (1)

Fix in Cursor Fix in Web

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.

2 participants