From af3207bd8b0d44724d6a33983b466833b8ed34e7 Mon Sep 17 00:00:00 2001 From: Andrey Epin Date: Tue, 19 Mar 2024 15:56:26 -0700 Subject: Update modify share action on selection change update 1. Trivial refactoring: move repetitive displayModifyShareAction call from each ConventPreviewUi#display implenentation into ChooserContentPreviewUi#displayContentPreview. 2. Trivial refactoring: add new dependency to ChooserContentPreviewUi for the modify share action factory but still use ChooserActionFactory to provide the action. 3. Use a new factory for the modify share action that alway reads the lates ChooserRequest values. Update the modify share action upon ChooserRequest changes. Bug: 302691505 Test: manual functionality test with the ShareTest app Flag: ACONFIG android.service.chooser.chooser_payload_toggling DEVELOPMENT Change-Id: I3ee55746387bc8ba413244b76aca374a361d696d --- .../android/intentresolver/ChooserActivity.java | 4 ++- .../contentpreview/ChooserContentPreviewUi.java | 31 ++++++++++++++-- .../contentpreview/ContentPreviewUi.java | 23 ++++++------ .../contentpreview/FileContentPreviewUi.java | 5 +-- .../FilesPlusTextContentPreviewUi.java | 5 +-- .../contentpreview/ShareouselContentPreviewUi.kt | 10 ++---- .../contentpreview/TextContentPreviewUi.java | 5 +-- .../contentpreview/UnifiedContentPreviewUi.java | 5 +-- .../intentresolver/v2/ChooserActionFactory.java | 41 +++++++++------------- .../android/intentresolver/v2/ChooserActivity.java | 27 ++++++++++---- 10 files changed, 86 insertions(+), 70 deletions(-) (limited to 'java') diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java index e36e9df3..9557b25b 100644 --- a/java/src/com/android/intentresolver/ChooserActivity.java +++ b/java/src/com/android/intentresolver/ChooserActivity.java @@ -307,12 +307,14 @@ public class ChooserActivity extends Hilt_ChooserActivity implements mChooserRequest.getTargetIntent(), /*additionalContentUri = */ null, /*isPayloadTogglingEnabled = */ false); + final ChooserActionFactory chooserActionFactory = createChooserActionFactory(); mChooserContentPreviewUi = new ChooserContentPreviewUi( getCoroutineScope(getLifecycle()), previewViewModel.getPreviewDataProvider(), mChooserRequest.getTargetIntent(), previewViewModel.getImageLoader(), - createChooserActionFactory(), + chooserActionFactory, + chooserActionFactory::getModifyShareAction, mEnterTransitionAnimationDelegate, new HeadlineGeneratorImpl(this), ContentTypeHint.NONE, diff --git a/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java b/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java index 6f201ad5..67458697 100644 --- a/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java +++ b/java/src/com/android/intentresolver/contentpreview/ChooserContentPreviewUi.java @@ -39,6 +39,7 @@ import com.android.intentresolver.widget.ImagePreviewView.TransitionElementStatu import java.util.List; import java.util.function.Consumer; +import java.util.function.Supplier; import kotlinx.coroutines.CoroutineScope; @@ -77,7 +78,9 @@ public final class ChooserContentPreviewUi { * Provides a share modification action, if any. */ @Nullable - ActionRow.Action getModifyShareAction(); + default ActionRow.Action getModifyShareAction() { + return null; + } /** *

@@ -93,6 +96,9 @@ public final class ChooserContentPreviewUi { @VisibleForTesting final ContentPreviewUi mContentPreviewUi; + private final Supplier mModifyShareActionFactory; + @Nullable + private View mHeadlineParent; public ChooserContentPreviewUi( CoroutineScope scope, @@ -100,6 +106,7 @@ public final class ChooserContentPreviewUi { Intent targetIntent, ImageLoader imageLoader, ActionFactory actionFactory, + Supplier modifyShareActionFactory, TransitionElementStatusCallback transitionElementStatusCallback, HeadlineGenerator headlineGenerator, ContentTypeHint contentTypeHint, @@ -108,6 +115,7 @@ public final class ChooserContentPreviewUi { boolean isPayloadTogglingEnabled) { mScope = scope; mIsPayloadTogglingEnabled = isPayloadTogglingEnabled; + mModifyShareActionFactory = modifyShareActionFactory; mContentPreviewUi = createContentPreview( previewData, targetIntent, @@ -162,7 +170,7 @@ public final class ChooserContentPreviewUi { if (previewType == CONTENT_PREVIEW_PAYLOAD_SELECTION && mIsPayloadTogglingEnabled) { transitionElementStatusCallback.onAllTransitionElementsReady(); // TODO - return new ShareouselContentPreviewUi(actionFactory); + return new ShareouselContentPreviewUi(); } boolean isSingleImageShare = previewData.getUriCount() == 1 @@ -220,7 +228,24 @@ public final class ChooserContentPreviewUi { ViewGroup parent, @Nullable View headlineViewParent) { - return mContentPreviewUi.display(resources, layoutInflater, parent, headlineViewParent); + ViewGroup layout = + mContentPreviewUi.display(resources, layoutInflater, parent, headlineViewParent); + mHeadlineParent = headlineViewParent == null ? layout : headlineViewParent; + if (mHeadlineParent != null) { + ContentPreviewUi.displayModifyShareAction( + mHeadlineParent, mModifyShareActionFactory.get()); + } + return layout; + } + + /** + * Update Modify Share Action, if it is inflated. + */ + public void updateModifyShareAction() { + if (mHeadlineParent != null) { + ContentPreviewUi.displayModifyShareAction( + mHeadlineParent, mModifyShareActionFactory.get()); + } } private static TextContentPreviewUi createTextPreview( diff --git a/java/src/com/android/intentresolver/contentpreview/ContentPreviewUi.java b/java/src/com/android/intentresolver/contentpreview/ContentPreviewUi.java index b0fb278e..71d5fc0b 100644 --- a/java/src/com/android/intentresolver/contentpreview/ContentPreviewUi.java +++ b/java/src/com/android/intentresolver/contentpreview/ContentPreviewUi.java @@ -98,16 +98,19 @@ public abstract class ContentPreviewUi { } } - protected static void displayModifyShareAction( - View layout, ChooserContentPreviewUi.ActionFactory actionFactory) { - ActionRow.Action modifyShareAction = actionFactory.getModifyShareAction(); - if (modifyShareAction != null && layout != null) { - TextView modifyShareView = layout.findViewById(R.id.reselection_action); - if (modifyShareView != null) { - modifyShareView.setText(modifyShareAction.getLabel()); - modifyShareView.setVisibility(View.VISIBLE); - modifyShareView.setOnClickListener(view -> modifyShareAction.getOnClicked().run()); - } + static void displayModifyShareAction( + View layout, @Nullable ActionRow.Action modifyShareAction) { + TextView modifyShareView = + layout == null ? null : layout.findViewById(R.id.reselection_action); + if (modifyShareView == null) { + return; + } + if (modifyShareAction != null) { + modifyShareView.setText(modifyShareAction.getLabel()); + modifyShareView.setVisibility(View.VISIBLE); + modifyShareView.setOnClickListener(view -> modifyShareAction.getOnClicked().run()); + } else { + modifyShareView.setVisibility(View.GONE); } } diff --git a/java/src/com/android/intentresolver/contentpreview/FileContentPreviewUi.java b/java/src/com/android/intentresolver/contentpreview/FileContentPreviewUi.java index d4eea8b9..d127d929 100644 --- a/java/src/com/android/intentresolver/contentpreview/FileContentPreviewUi.java +++ b/java/src/com/android/intentresolver/contentpreview/FileContentPreviewUi.java @@ -77,10 +77,7 @@ class FileContentPreviewUi extends ContentPreviewUi { LayoutInflater layoutInflater, ViewGroup parent, @Nullable View headlineViewParent) { - ViewGroup layout = displayInternal(resources, layoutInflater, parent, headlineViewParent); - displayModifyShareAction( - headlineViewParent == null ? layout : headlineViewParent, mActionFactory); - return layout; + return displayInternal(resources, layoutInflater, parent, headlineViewParent); } private ViewGroup displayInternal( diff --git a/java/src/com/android/intentresolver/contentpreview/FilesPlusTextContentPreviewUi.java b/java/src/com/android/intentresolver/contentpreview/FilesPlusTextContentPreviewUi.java index 6832c5c4..4758534d 100644 --- a/java/src/com/android/intentresolver/contentpreview/FilesPlusTextContentPreviewUi.java +++ b/java/src/com/android/intentresolver/contentpreview/FilesPlusTextContentPreviewUi.java @@ -109,10 +109,7 @@ class FilesPlusTextContentPreviewUi extends ContentPreviewUi { LayoutInflater layoutInflater, ViewGroup parent, @Nullable View headlineViewParent) { - ViewGroup layout = displayInternal(layoutInflater, parent, headlineViewParent); - displayModifyShareAction( - headlineViewParent == null ? layout : headlineViewParent, mActionFactory); - return layout; + return displayInternal(layoutInflater, parent, headlineViewParent); } public void updatePreviewMetadata(List files) { diff --git a/java/src/com/android/intentresolver/contentpreview/ShareouselContentPreviewUi.kt b/java/src/com/android/intentresolver/contentpreview/ShareouselContentPreviewUi.kt index 463da5fa..3530ede1 100644 --- a/java/src/com/android/intentresolver/contentpreview/ShareouselContentPreviewUi.kt +++ b/java/src/com/android/intentresolver/contentpreview/ShareouselContentPreviewUi.kt @@ -30,15 +30,12 @@ import androidx.compose.ui.platform.ComposeView import androidx.compose.ui.platform.LocalContext import androidx.lifecycle.viewmodel.compose.viewModel import com.android.intentresolver.R -import com.android.intentresolver.contentpreview.ChooserContentPreviewUi.ActionFactory import com.android.intentresolver.contentpreview.payloadtoggle.ui.composable.Shareousel import com.android.intentresolver.contentpreview.payloadtoggle.ui.viewmodel.ShareouselViewModel import com.android.intentresolver.v2.ui.viewmodel.ChooserViewModel @VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE) -class ShareouselContentPreviewUi( - private val actionFactory: ActionFactory, -) : ContentPreviewUi() { +class ShareouselContentPreviewUi : ContentPreviewUi() { override fun getType(): Int = ContentPreviewType.CONTENT_PREVIEW_IMAGE @@ -47,10 +44,7 @@ class ShareouselContentPreviewUi( layoutInflater: LayoutInflater, parent: ViewGroup, headlineViewParent: View?, - ): ViewGroup = - displayInternal(parent, headlineViewParent).also { layout -> - displayModifyShareAction(headlineViewParent ?: layout, actionFactory) - } + ): ViewGroup = displayInternal(parent, headlineViewParent) private fun displayInternal(parent: ViewGroup, headlineViewParent: View?): ViewGroup { if (headlineViewParent != null) { diff --git a/java/src/com/android/intentresolver/contentpreview/TextContentPreviewUi.java b/java/src/com/android/intentresolver/contentpreview/TextContentPreviewUi.java index fbdc5853..a7ae81b0 100644 --- a/java/src/com/android/intentresolver/contentpreview/TextContentPreviewUi.java +++ b/java/src/com/android/intentresolver/contentpreview/TextContentPreviewUi.java @@ -83,10 +83,7 @@ class TextContentPreviewUi extends ContentPreviewUi { LayoutInflater layoutInflater, ViewGroup parent, @Nullable View headlineViewParent) { - ViewGroup layout = displayInternal(layoutInflater, parent, headlineViewParent); - displayModifyShareAction( - headlineViewParent == null ? layout : headlineViewParent, mActionFactory); - return layout; + return displayInternal(layoutInflater, parent, headlineViewParent); } private ViewGroup displayInternal( diff --git a/java/src/com/android/intentresolver/contentpreview/UnifiedContentPreviewUi.java b/java/src/com/android/intentresolver/contentpreview/UnifiedContentPreviewUi.java index 0974c79b..b248e429 100644 --- a/java/src/com/android/intentresolver/contentpreview/UnifiedContentPreviewUi.java +++ b/java/src/com/android/intentresolver/contentpreview/UnifiedContentPreviewUi.java @@ -94,10 +94,7 @@ class UnifiedContentPreviewUi extends ContentPreviewUi { LayoutInflater layoutInflater, ViewGroup parent, @Nullable View headlineViewParent) { - ViewGroup layout = displayInternal(layoutInflater, parent, headlineViewParent); - displayModifyShareAction( - headlineViewParent == null ? layout : headlineViewParent, mActionFactory); - return layout; + return displayInternal(layoutInflater, parent, headlineViewParent); } private void setFiles(List files) { diff --git a/java/src/com/android/intentresolver/v2/ChooserActionFactory.java b/java/src/com/android/intentresolver/v2/ChooserActionFactory.java index 9077a18d..efd5bfd1 100644 --- a/java/src/com/android/intentresolver/v2/ChooserActionFactory.java +++ b/java/src/com/android/intentresolver/v2/ChooserActionFactory.java @@ -102,7 +102,6 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio @Nullable private Runnable mCopyButtonRunnable; private Runnable mEditButtonRunnable; private final ImmutableList mCustomActions; - @Nullable private final ChooserAction mModifyShareAction; private final Consumer mExcludeSharedTextAction; @Nullable private final ShareResultSender mShareResultSender; private final Consumer mFinishCallback; @@ -124,7 +123,6 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio Intent targetIntent, String referrerPackageName, List chooserActions, - @Nullable ChooserAction modifyShareAction, Optional imageEditor, EventLog log, Consumer onUpdateSharedTextIsExcluded, @@ -150,7 +148,6 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio activityStarter, log), chooserActions, - modifyShareAction, onUpdateSharedTextIsExcluded, log, shareResultSender, @@ -164,7 +161,6 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio @Nullable Runnable copyButtonRunnable, Runnable editButtonRunnable, List customActions, - @Nullable ChooserAction modifyShareAction, Consumer onUpdateSharedTextIsExcluded, EventLog log, @Nullable ShareResultSender shareResultSender, @@ -173,7 +169,6 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio mCopyButtonRunnable = copyButtonRunnable; mEditButtonRunnable = editButtonRunnable; mCustomActions = ImmutableList.copyOf(customActions); - mModifyShareAction = modifyShareAction; mExcludeSharedTextAction = onUpdateSharedTextIsExcluded; mLog = log; mShareResultSender = shareResultSender; @@ -212,7 +207,11 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio for (int i = 0; i < mCustomActions.size(); i++) { final int position = i; ActionRow.Action actionRow = createCustomAction( - mCustomActions.get(i), () -> logCustomAction(position)); + mContext, + mCustomActions.get(i), + () -> logCustomAction(position), + mShareResultSender, + mFinishCallback); if (actionRow != null) { actions.add(actionRow); } @@ -220,15 +219,6 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio return actions; } - /** - * Provides a share modification action, if any. - */ - @Override - @Nullable - public ActionRow.Action getModifyShareAction() { - return createCustomAction(mModifyShareAction, this::logModifyShareAction); - } - /** *

* Creates an exclude-text action that can be called when the user changes shared text @@ -360,11 +350,16 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio } @Nullable - ActionRow.Action createCustomAction(@Nullable ChooserAction action, Runnable loggingRunnable) { + static ActionRow.Action createCustomAction( + Context context, + @Nullable ChooserAction action, + Runnable loggingRunnable, + ShareResultSender shareResultSender, + Consumer finishCallback) { if (action == null) { return null; } - Drawable icon = action.getIcon().loadDrawable(mContext); + Drawable icon = action.getIcon().loadDrawable(context); if (icon == null && TextUtils.isEmpty(action.getLabel())) { return null; } @@ -381,7 +376,7 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio null, null, ActivityOptions.makeCustomAnimation( - mContext, + context, R.anim.slide_in_right, R.anim.slide_out_left) .toBundle()); @@ -391,10 +386,10 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio if (loggingRunnable != null) { loggingRunnable.run(); } - if (mShareResultSender != null) { - mShareResultSender.onActionSelected(ShareAction.APPLICATION_DEFINED); + if (shareResultSender != null) { + shareResultSender.onActionSelected(ShareAction.APPLICATION_DEFINED); } - mFinishCallback.accept(Activity.RESULT_OK); + finishCallback.accept(Activity.RESULT_OK); } ); } @@ -402,8 +397,4 @@ public final class ChooserActionFactory implements ChooserContentPreviewUi.Actio void logCustomAction(int position) { mLog.logCustomActionSelected(position); } - - private void logModifyShareAction() { - mLog.logActionSelected(EventLog.SELECTION_TYPE_MODIFY_SHARE); - } } diff --git a/java/src/com/android/intentresolver/v2/ChooserActivity.java b/java/src/com/android/intentresolver/v2/ChooserActivity.java index b164bd9f..cb97d94f 100644 --- a/java/src/com/android/intentresolver/v2/ChooserActivity.java +++ b/java/src/com/android/intentresolver/v2/ChooserActivity.java @@ -153,6 +153,7 @@ import com.android.intentresolver.v2.ui.ShareResultSenderFactory; import com.android.intentresolver.v2.ui.model.ActivityModel; import com.android.intentresolver.v2.ui.model.ChooserRequest; import com.android.intentresolver.v2.ui.viewmodel.ChooserViewModel; +import com.android.intentresolver.widget.ActionRow; import com.android.intentresolver.widget.ImagePreviewView; import com.android.intentresolver.widget.ResolverDrawerLayout; import com.android.internal.annotations.VisibleForTesting; @@ -613,6 +614,7 @@ public class ChooserActivity extends Hilt_ChooserActivity implements mRequest.getTargetIntent(), previewViewModel.getImageLoader(), createChooserActionFactory(), + createModifyShareActionFactory(), mEnterTransitionAnimationDelegate, new HeadlineGeneratorImpl(this), mRequest.getContentTypeHint(), @@ -664,6 +666,7 @@ public class ChooserActivity extends Hilt_ChooserActivity implements boolean recreateAdapters = shouldUpdateAdapters(mRequest, chooserRequest); mRequest = chooserRequest; updateShareResultSender(); + mChooserContentPreviewUi.updateModifyShareAction(); if (recreateAdapters) { recreatePagerAdapter(); } @@ -2104,7 +2107,6 @@ public class ChooserActivity extends Hilt_ChooserActivity implements mRequest.getTargetIntent(), mRequest.getLaunchedFromPackage(), mRequest.getChooserActions(), - mRequest.getModifyShareAction(), mImageEditor, getEventLog(), (isExcluded) -> mExcludeSharedText = isExcluded, @@ -2134,15 +2136,26 @@ public class ChooserActivity extends Hilt_ChooserActivity implements } }, mShareResultSender, - (status) -> { - if (status != null) { - setResult(status); - } - finish(); - }, + this::finishWithStatus, mClipboardManager); } + private Supplier createModifyShareActionFactory() { + return () -> ChooserActionFactory.createCustomAction( + ChooserActivity.this, + mRequest.getModifyShareAction(), + () -> getEventLog().logActionSelected(EventLog.SELECTION_TYPE_MODIFY_SHARE), + mShareResultSender, + this::finishWithStatus); + } + + private void finishWithStatus(@Nullable Integer status) { + if (status != null) { + setResult(status); + } + finish(); + } + /* * Need to dynamically adjust how many icons can fit per row before we add them, * which also means setting the correct offset to initially show the content -- cgit v1.2.3-59-g8ed1b