-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Duck.ai: Contextual Sheet UX #7384
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
base: develop
Are you sure you want to change the base?
Conversation
be969ee to
0352de0
Compare
0352de0 to
f8ee4ca
Compare
...duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/DuckChatContextualBottomSheet.kt
Outdated
Show resolved
Hide resolved
| duckChatContextualSheet = duckChatContextualBottomSheetFactory.create() | ||
| } | ||
| duckChatContextualSheet?.show(childFragmentManager, DuckChatContextualBottomSheet.TAG) | ||
| } |
There was a problem hiding this comment.
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)
app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt
Outdated
Show resolved
Hide resolved
| requestFileDownload(url, contentDisposition, mimeType) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
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)
android-design-system/design-system/src/main/res/values/design-system-theming.xml
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| val url = arguments?.getString(KEY_DUCK_AI_URL) ?: "https://duckduckgo.com/?q=DuckDuckGo+AI+Chat&ia=chat&duckai=5" | ||
| binding.simpleWebview.loadUrl(url) |
There was a problem hiding this comment.
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.
|
|
||
| private var pendingFileDownload: PendingFileDownload? = null | ||
| private val downloadMessagesJob = ConflatedJob() | ||
| private var isExpanded = false |
There was a problem hiding this comment.
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.

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
Enable Contextual FF - NTP
Disable Contextual FF
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.
DuckChatContextualBottomSheetwith prompt input + embedded WebView, downloads, and JS messaging.DuckChatContextualBottomSheetFactoryto create/show the sheet fromBrowserTabFragment.Command.ShowDuckAIContextualModeand handles it inBrowserTabFragment.BrowserTabViewModel.onDuckChatOmnibarButtonClickednow branches:showContextualMode(non‑NTP) → show sheet;showFullScreenMode→ navigate; else → open legacy chat.contextualMode()added/updated inDuckChatFeature(default FALSE).BrowserTabViewModelTestvalidating contextual vs full‑screen/NTP behavior.duckchat/.../bottom_sheet_duck_ai_contextual.xml, styles, strings, dimens.duck_ai_prompt_background.xml,ic_arrow_down_right_16.xml,ic_duck_ai_color_24.xml,ic_expand_24.xml).DuckChatWebViewFragment(exposesREQUEST_CODE_CHOOSE_FILE).Written by Cursor Bugbot for commit bb721d4. This will update automatically on new commits. Configure here.