-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Update widgets to match the changes in DuckChat interface #6624
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
Conversation
… update widget configuration for Duck.ai support.
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
val voiceSearchEnabled = withContext(dispatcherProvider.io()) { | ||
voiceSearchAvailability.isVoiceSearchAvailable | ||
val (voiceSearchEnabled, duckAiEnabled) = withContext(dispatcherProvider.io()) { | ||
voiceSearchAvailability.isVoiceSearchAvailable to (duckChat.isEnabled() && duckChat.wasOpenedBefore()) |
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.
Updated to use duckChat.isEnabled()
which was not available before.
* | ||
* @return Intent that can be used to open DuckChat, or null if DuckChat cannot be opened. | ||
*/ | ||
suspend fun createDuckChatIntent(): Intent? |
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.
Removed as it's not needed anymore.
@@ -380,21 +380,6 @@ class RealDuckChat @Inject constructor( | |||
openDuckChat(emptyMap()) | |||
} | |||
|
|||
override suspend fun createDuckChatIntent(): Intent? { |
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.
Removed as it's not needed anymore.
): PendingIntent { | ||
val intent = BrowserActivity.intent(context, openDuckChat = true).also { it.action = Intent.ACTION_VIEW } |
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.
Built the intent directly here.
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.
Pull Request Overview
This PR refactors the DuckChat widget integration by removing the indirect createDuckChatIntent()
method and replacing it with a more direct approach using BrowserActivity.intent()
with an openDuckChat
parameter.
Key Changes:
- Removed the
createDuckChatIntent()
method from both the DuckChat interface and implementation - Updated the SearchWidgetConfigurator to use
BrowserActivity.intent(openDuckChat = true)
directly - Simplified the widget configuration logic by combining multiple async calls into a single context switch
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
duckchat-api/DuckChat.kt |
Removed the createDuckChatIntent() method from the interface and its documentation |
duckchat-impl/RealDuckChat.kt |
Removed the createDuckChatIntent() implementation method |
SearchWidgetConfigurator.kt |
Updated to use BrowserActivity.intent() directly and streamlined async operations |
): PendingIntent { | ||
val intent = BrowserActivity.intent(context, openDuckChat = true).also { it.action = Intent.ACTION_VIEW } |
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.
[nitpick] The intent creation and action setting should be separated for better readability. Consider creating the intent first, then setting the action on a separate line.
val intent = BrowserActivity.intent(context, openDuckChat = true).also { it.action = Intent.ACTION_VIEW } | |
val intent = BrowserActivity.intent(context, openDuckChat = true) | |
intent.action = Intent.ACTION_VIEW |
Copilot uses AI. Check for mistakes.
val voiceSearchEnabled = withContext(dispatcherProvider.io()) { | ||
voiceSearchAvailability.isVoiceSearchAvailable | ||
val (voiceSearchEnabled, duckAiEnabled) = withContext(dispatcherProvider.io()) { | ||
voiceSearchAvailability.isVoiceSearchAvailable to (duckChat.isEnabled() && duckChat.wasOpenedBefore()) |
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.
[nitpick] The complex boolean expression for duckAiEnabled should be extracted to improve readability. Consider creating a separate variable or method for the DuckChat availability check.
Copilot uses AI. Check for mistakes.
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.
LGTM though our AI overlords are not happy 😂
@@ -41,13 +42,9 @@ class SearchWidgetConfigurator @Inject constructor( | |||
remoteViews: RemoteViews, | |||
fromFavWidget: Boolean, | |||
) { | |||
val voiceSearchEnabled = withContext(dispatcherProvider.io()) { | |||
voiceSearchAvailability.isVoiceSearchAvailable | |||
val (voiceSearchEnabled, duckAiEnabled) = withContext(dispatcherProvider.io()) { |
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.
❤️
36ca94d
into
feature/ana/add_duck_ai_entry_points_to_widgets_plus_additional_widgets
Task/Issue URL: https://app.asana.com/1/137249556945/project/1200581511062568/task/1211103096551754?focus=true
Description
Refactored the DuckChat widget integration to simplify the process of opening DuckChat from the search widget.
Removed the
createDuckChatIntent()
method from the DuckChat interface and implementation, replacing it with a more direct approach that usesBrowserActivity.intent()
with anopenDuckChat
parameter.Steps to test this PR
Code review.
NO UI changes