diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt index a9af4a869666..7731d09efd68 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt @@ -3108,7 +3108,13 @@ class BrowserTabFragment : val anchor = duckChatButtonAnchor duckChatButtonAnchor = null viewLifecycleOwner.lifecycleScope.launch(dispatchers.main()) { - duckChatContextual.launch(tabId, sourceUrl, anchor) { showDuckChatContextualSheet(tabId) } + duckChatContextual.launch(tabId, sourceUrl, anchor) { + if (sourceUrl == null) { + viewModel.openDuckChatFromOmnibar(query = null, hasFocus = false, isNtp = true) + } else { + showDuckChatContextualSheet(tabId) + } + } } } is Command.StartAddressBarTrackersAnimation -> { diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index 97e4517e46a0..9febf504706c 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -5897,37 +5897,49 @@ class BrowserTabViewModel @Inject constructor( } when { - // Contextual chat is about the page you're viewing, so it's only offered from the - // unfocused omnibar. Once the omnibar is focused (composing), fall through to full-screen - // Duck.ai. - duckAiFeatureState.showContextualMode.value && !isNtp && !hasFocus -> { + // Contextual chat is about the page you're viewing, so on a page it's only offered from + // the unfocused omnibar: once focused (composing), fall through to full-screen Duck.ai. + // The NTP omnibar is focused from the start, so there the menu hangs off an empty query + // instead, and only when it carries the Chats entry alongside New Chat. + duckAiFeatureState.showContextualMode.value && + if (isNtp) query.isNullOrBlank() else !hasFocus -> { command.value = Command.ShowDuckAIContextualMode(tabId, url) } - else -> { - val (url, submittedAiPrompt) = when { - hasFocus && isNtp && query.isNullOrBlank() -> duckChat.getDuckChatUrl(query ?: "", false) to false - hasFocus && queryUrlPredictor.isUrl(query ?: "") -> (query ?: "") to false - hasFocus -> duckChat.getDuckChatUrl(query ?: "", true) to !query.isNullOrBlank() - else -> duckChat.getDuckChatUrl(query ?: "", false) to false - } - if (duckChat.isDuckChatUrl(url.toUri())) { - if (submittedAiPrompt) { - browserInteractionsPlugins.getPlugins().forEach { - it.onAiPromptSubmitted(source = DuckChatEntryPoint.ADDRESS_BAR_ICON.name.lowercase()) - } - } - duckChat.reportDuckChatEntry( - DuckChatEntryPoint.ADDRESS_BAR_ICON, - opensNewTab = false, - hasPrompt = submittedAiPrompt, - ) - submitQuery(url, QueryOrigin.FromUser, QuerySubmissionSource.INTERNAL_NAVIGATION) - } else { - // The typed-URL branch above: genuinely a URL submission, not a Duck.ai one. - onUserSubmittedQuery(url) + else -> openDuckChatFromOmnibar(query, hasFocus, isNtp) + } + } + + /** + * Opens Duck.ai straight from the address bar icon. Also the fallback when the NTP menu declines + * to show, so that path keeps behaving exactly as it did before the menu existed. + */ + fun openDuckChatFromOmnibar( + query: String?, + hasFocus: Boolean, + isNtp: Boolean, + ) { + val (url, submittedAiPrompt) = when { + hasFocus && isNtp && query.isNullOrBlank() -> duckChat.getDuckChatUrl(query ?: "", false) to false + hasFocus && queryUrlPredictor.isUrl(query ?: "") -> (query ?: "") to false + hasFocus -> duckChat.getDuckChatUrl(query ?: "", true) to !query.isNullOrBlank() + else -> duckChat.getDuckChatUrl(query ?: "", false) to false + } + if (duckChat.isDuckChatUrl(url.toUri())) { + if (submittedAiPrompt) { + browserInteractionsPlugins.getPlugins().forEach { + it.onAiPromptSubmitted(source = DuckChatEntryPoint.ADDRESS_BAR_ICON.name.lowercase()) } } + duckChat.reportDuckChatEntry( + DuckChatEntryPoint.ADDRESS_BAR_ICON, + opensNewTab = false, + hasPrompt = submittedAiPrompt, + ) + submitQuery(url, QueryOrigin.FromUser, QuerySubmissionSource.INTERNAL_NAVIGATION) + } else { + // The typed-URL branch above: genuinely a URL submission, not a Duck.ai one. + onUserSubmittedQuery(url) } } diff --git a/app/src/test/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/test/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index 605b4d0229cf..e1d92ad10d6d 100644 --- a/app/src/test/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/test/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -10142,6 +10142,37 @@ class BrowserTabViewModelTest { assertTrue(commandCaptor.allValues.any { it is Command.ShowDuckAIContextualMode }) } + @Test + fun whenOnDuckChatOmnibarButtonClickedOnNtpWithBlankQueryThenShowsContextualMenu() { + mockDuckAiContextualModeFlow.value = true + + testee.onDuckChatOmnibarButtonClicked(query = null, hasFocus = true, isNtp = true) + + verify(mockCommandObserver, atLeastOnce()).onChanged(commandCaptor.capture()) + assertTrue(commandCaptor.allValues.any { it is Command.ShowDuckAIContextualMode }) + } + + @Test + fun whenOnDuckChatOmnibarButtonClickedOnNtpWithTypedQueryThenOpensDuckChatNotMenu() { + mockDuckAiContextualModeFlow.value = true + whenever(mockOmnibarConverter.convertQueryToUrl(duckChatURL, null)).thenReturn(duckChatURL) + + testee.onDuckChatOmnibarButtonClicked(query = "example", hasFocus = true, isNtp = true) + + verify(mockDuckChat).getDuckChatUrl(eq("example"), eq(true), any()) + verify(mockCommandObserver, atLeastOnce()).onChanged(commandCaptor.capture()) + assertFalse(commandCaptor.allValues.any { it is Command.ShowDuckAIContextualMode }) + } + + @Test + fun whenOpenDuckChatFromOmnibarOnNtpThenOpensDuckChatWithoutPrompt() { + whenever(mockOmnibarConverter.convertQueryToUrl(duckChatURL, null)).thenReturn(duckChatURL) + + testee.openDuckChatFromOmnibar(query = null, hasFocus = false, isNtp = true) + + verify(mockDuckChat).getDuckChatUrl(eq(""), eq(false), any()) + } + @Test fun whenOnDuckChatOmnibarButtonClickedFocusedWithContextualModeThenOpensDuckChatNotContextualSheet() { mockDuckAiContextualModeFlow.value = true diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/RealDuckChatContextual.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/RealDuckChatContextual.kt index 2e60be6dc900..5172ad952ad7 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/RealDuckChatContextual.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/contextual/RealDuckChatContextual.kt @@ -63,6 +63,12 @@ class RealDuckChatContextual @Inject constructor( showChatSurface() return } + if (sourceUrl == null && !duckChatInternal.isContextualMenuAllChatsEnabled()) { + // Nothing to ask about and no Chats entry, so a one-item menu would be worse than the + // caller's own fallback (opening Duck.ai). + showChatSurface() + return + } if (hasChatInProgress(sourceTabId)) { // The sheet would reopen the existing chat for this tab, so skip the entry menu and open it directly. showChatSurface() @@ -71,7 +77,7 @@ class RealDuckChatContextual @Inject constructor( ?.takeIf { duckDuckGoUrlDetector.isDuckDuckGoQueryUrl(it) } ?.let { duckDuckGoUrlDetector.extractQuery(it) } ?.takeIf { it.isNotBlank() } - showMenu(sourceTabId, anchor, serpQuery, showChatSurface) + showMenu(sourceTabId, anchor, sourceUrl, serpQuery, showChatSurface) } } @@ -106,6 +112,7 @@ class RealDuckChatContextual @Inject constructor( private fun showMenu( sourceTabId: String, anchor: View, + sourceUrl: String?, serpQuery: String?, onAskAboutPage: () -> Unit, ) { @@ -117,7 +124,10 @@ class RealDuckChatContextual @Inject constructor( openNewChatTab(activity, sourceTabId) } val askItem = content.findViewById(R.id.contextualChatMenuAskAboutPage) - if (serpQuery != null) { + if (sourceUrl == null) { + // No page open (NTP or a blank tab), so there is nothing to ask about. + askItem.gone() + } else if (serpQuery != null) { askItem.setPrimaryText(activity.getString(R.string.duckChatContextualAskAboutSearch)) popup.onMenuItemClicked(askItem) { duckChatPixels.reportContextualAddressBarMenuAskAboutSearchSelected() diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/nativeinput/NativeInputPlugin.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/nativeinput/NativeInputPlugin.kt index 0e2592e678cb..4a0cea9d9ee0 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/nativeinput/NativeInputPlugin.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/nativeinput/NativeInputPlugin.kt @@ -31,6 +31,12 @@ interface NativeInputHost { /** Submit the current input as a chat message; opens a new chat session if the input is empty. */ fun submit() + /** `true` when the input carries nothing submittable, i.e. [submit] would start an empty chat. */ + fun isInputEmpty(): Boolean + + /** The tab the widget is currently configured for, or null before it has been configured. */ + fun tabId(): String? + /** Stop the active chat stream. Delegates to the host's [NativeInputWidget.onStopTapped] callback. */ fun stop() diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/nativeinput/plugins/StartChatNativeInputPlugin.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/nativeinput/plugins/StartChatNativeInputPlugin.kt index b702eb55b3ea..0eafe74f4966 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/nativeinput/plugins/StartChatNativeInputPlugin.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/nativeinput/plugins/StartChatNativeInputPlugin.kt @@ -37,6 +37,6 @@ class StartChatNativeInputPlugin @Inject constructor() : NativeInputPlugin { override val containerId: Int = R.id.startChatContainer override fun createView(context: Context, host: NativeInputHost): View = StartChatView(context).apply { - onIconClicked = { host.submit() } + this.host = host } } diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/nativeinput/views/NativeInputModeWidget.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/nativeinput/views/NativeInputModeWidget.kt index 7124dc5e298e..004e4617c3f0 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/nativeinput/views/NativeInputModeWidget.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/nativeinput/views/NativeInputModeWidget.kt @@ -2072,6 +2072,10 @@ class NativeInputModeWidget @JvmOverloads constructor( } } + override fun isInputEmpty(): Boolean = inputField.text.getTextToSubmit() == null + + override fun tabId(): String? = activeTabId + override fun stop() { // Single chokepoint for every stop affordance (the streaming-plugin button routes here via // host.stop(), and the input-screen stop button calls stop() too), so the pixel fires once. diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/nativeinput/views/StartChatView.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/nativeinput/views/StartChatView.kt index aa4dcc89fd2c..5d67f2564607 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/nativeinput/views/StartChatView.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/nativeinput/views/StartChatView.kt @@ -29,11 +29,14 @@ import androidx.lifecycle.lifecycleScope import com.duckduckgo.anvil.annotations.InjectWith import com.duckduckgo.common.utils.ViewViewModelFactory import com.duckduckgo.di.scopes.ViewScope +import com.duckduckgo.duckchat.api.DuckChatContextual import com.duckduckgo.duckchat.impl.R +import com.duckduckgo.duckchat.impl.nativeinput.NativeInputHost import dagger.android.support.AndroidSupportInjection import kotlinx.coroutines.Job import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.launch import javax.inject.Inject @InjectWith(ViewScope::class) @@ -45,6 +48,8 @@ class StartChatView @JvmOverloads constructor( @Inject lateinit var viewModelFactory: ViewViewModelFactory + @Inject lateinit var duckChatContextual: DuckChatContextual + private val viewModel by lazy { ViewModelProvider(findViewTreeViewModelStoreOwner()!!, viewModelFactory)[StartChatViewModel::class.java] } @@ -52,7 +57,7 @@ class StartChatView @JvmOverloads constructor( private val icon: ImageView by lazy { findViewById(R.id.aiChatIconMenu) } private var visibilityJob: Job? = null - var onIconClicked: (() -> Unit)? = null + var host: NativeInputHost? = null init { inflate(context, R.layout.view_start_chat, this) @@ -61,7 +66,7 @@ class StartChatView @JvmOverloads constructor( override fun onAttachedToWindow() { AndroidSupportInjection.inject(this) super.onAttachedToWindow() - icon.setOnClickListener { onIconClicked?.invoke() } + icon.setOnClickListener { onIconTapped() } observeVisibility() } @@ -71,6 +76,20 @@ class StartChatView @JvmOverloads constructor( visibilityJob = null } + private fun onIconTapped() { + val host = host ?: return + val tabId = host.tabId() + val scope = findViewTreeLifecycleOwner()?.lifecycleScope + val showMenu = viewModel.onIconClicked(inputEmpty = host.isInputEmpty()) == StartChatViewModel.IconAction.SHOW_MENU + if (!showMenu || tabId == null || scope == null) { + host.submit() + return + } + // No page behind the new tab page, so the menu drops Ask About Page and offers New Chat and + // Chats. Submitting stays the fallback for when the menu itself declines to show. + scope.launch { duckChatContextual.launch(tabId, sourceUrl = null, anchor = icon) { host.submit() } } + } + private fun observeVisibility() { val scope = findViewTreeLifecycleOwner()?.lifecycleScope ?: return visibilityJob?.cancel() diff --git a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/nativeinput/views/StartChatViewModel.kt b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/nativeinput/views/StartChatViewModel.kt index ecd77cf6c1f6..5a1dd8d7cb2f 100644 --- a/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/nativeinput/views/StartChatViewModel.kt +++ b/duckchat/duckchat-impl/src/main/java/com/duckduckgo/duckchat/impl/ui/nativeinput/views/StartChatViewModel.kt @@ -31,10 +31,19 @@ import javax.inject.Inject @ContributesViewModel(ViewScope::class) class StartChatViewModel @Inject constructor( duckAiFeatureState: DuckAiFeatureState, - duckChatInternal: DuckChatInternal, + private val duckChatInternal: DuckChatInternal, nativeInputStateProvider: NativeInputStateProvider, ) : ViewModel() { + enum class IconAction { SUBMIT, SHOW_MENU } + + /** + * An empty input has nothing to send, so it offers the Duck.ai menu instead of starting a chat. + * Anything typed always goes straight to Duck.ai. + */ + fun onIconClicked(inputEmpty: Boolean): IconAction = + if (inputEmpty && duckChatInternal.isContextualMenuAllChatsEnabled()) IconAction.SHOW_MENU else IconAction.SUBMIT + /** * Show the start-chat icon only when Duck.ai is available (feature enabled + * user setting on), the address bar shortcut is enabled, but the input-screen diff --git a/duckchat/duckchat-impl/src/main/res/layout/popup_contextual_chat_menu.xml b/duckchat/duckchat-impl/src/main/res/layout/popup_contextual_chat_menu.xml index dce748fa8239..52afafbf00f8 100644 --- a/duckchat/duckchat-impl/src/main/res/layout/popup_contextual_chat_menu.xml +++ b/duckchat/duckchat-impl/src/main/res/layout/popup_contextual_chat_menu.xml @@ -50,6 +50,6 @@ android:layout_width="match_parent" android:layout_height="wrap_content" app:leadingIcon="@drawable/ic_chats_24" - app:primaryText="@string/duckChatContextualAllChats" /> + app:primaryText="@string/duck_ai_chat_history_title" /> diff --git a/duckchat/duckchat-impl/src/main/res/values/donottranslate.xml b/duckchat/duckchat-impl/src/main/res/values/donottranslate.xml index 90d7146a3854..1b53bb52c004 100644 --- a/duckchat/duckchat-impl/src/main/res/values/donottranslate.xml +++ b/duckchat/duckchat-impl/src/main/res/values/donottranslate.xml @@ -39,7 +39,4 @@ Turn Duck.ai On Keep Duck.ai Off - - - All Chats diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/contextual/RealDuckChatContextualTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/contextual/RealDuckChatContextualTest.kt index 4ccdca7ba7f3..c591a60608a2 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/contextual/RealDuckChatContextualTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/contextual/RealDuckChatContextualTest.kt @@ -77,6 +77,18 @@ class RealDuckChatContextualTest { verifyNoInteractions(browserNav) } + @Test + fun whenNoPageAndChatsEntryDisabledThenChatSurfaceShownInsteadOfMenu() = runTest { + whenever(duckChatInternal.isContextualSheetRedesignEnabled()).thenReturn(true) + whenever(duckChatInternal.isContextualMenuAllChatsEnabled()).thenReturn(false) + var askAboutPageCount = 0 + + testee.launch("tabId", sourceUrl = null, anchor = anchor) { askAboutPageCount++ } + + // A menu of just New Chat is worse than the caller's own fallback. + assertEquals(1, askAboutPageCount) + } + @Test fun whenChatInProgressThenAskAboutPageInvokedWithoutShowingMenu() = runTest { whenever(duckChatInternal.isContextualSheetRedesignEnabled()).thenReturn(true) @@ -92,6 +104,7 @@ class RealDuckChatContextualTest { @Test fun whenStoredChatSessionExpiredThenTreatedAsNoChatInProgress() = runTest { whenever(duckChatInternal.isContextualSheetRedesignEnabled()).thenReturn(true) + whenever(duckChatInternal.isContextualMenuAllChatsEnabled()).thenReturn(true) whenever(contextualDataStore.getTabChatUrl("tabId")).thenReturn("https://duckduckgo.com/?chatId=123") whenever(contextualDataStore.getTabClosedTimestamp("tabId")).thenReturn(0L) whenever(sessionTimeoutProvider.sessionTimeoutMillis()).thenReturn(1L) diff --git a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/ui/nativeinput/views/StartChatViewModelTest.kt b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/ui/nativeinput/views/StartChatViewModelTest.kt index d4add35fe16c..f356d186908f 100644 --- a/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/ui/nativeinput/views/StartChatViewModelTest.kt +++ b/duckchat/duckchat-impl/src/test/kotlin/com/duckduckgo/duckchat/impl/ui/nativeinput/views/StartChatViewModelTest.kt @@ -25,10 +25,12 @@ import com.duckduckgo.duckchat.api.nativeinput.NativeInputState.InputMode import com.duckduckgo.duckchat.api.nativeinput.NativeInputState.ToggleSelection import com.duckduckgo.duckchat.api.nativeinput.NativeInputStateProvider import com.duckduckgo.duckchat.impl.DuckChatInternal +import com.duckduckgo.duckchat.impl.ui.nativeinput.views.StartChatViewModel.IconAction import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Rule @@ -60,6 +62,27 @@ class StartChatViewModelTest { private val testee = StartChatViewModel(duckAiFeatureState, duckChatInternal, nativeInputStateProvider) + @Test + fun whenIconClickedWithEmptyInputAndMenuEnabledThenShowMenu() { + whenever(duckChatInternal.isContextualMenuAllChatsEnabled()).thenReturn(true) + + assertEquals(IconAction.SHOW_MENU, testee.onIconClicked(inputEmpty = true)) + } + + @Test + fun whenIconClickedWithEmptyInputAndMenuDisabledThenSubmit() { + whenever(duckChatInternal.isContextualMenuAllChatsEnabled()).thenReturn(false) + + assertEquals(IconAction.SUBMIT, testee.onIconClicked(inputEmpty = true)) + } + + @Test + fun whenIconClickedWithTypedInputThenSubmitEvenWithMenuEnabled() { + whenever(duckChatInternal.isContextualMenuAllChatsEnabled()).thenReturn(true) + + assertEquals(IconAction.SUBMIT, testee.onIconClicked(inputEmpty = false)) + } + @Test fun whenSearchOnlyModeAndSearchToggleAndDuckAiEnabledThenVisible() = runTest { featureShowSettingsFlow.value = true