Commit 7e126100 authored by Luca Hunkeler's avatar Luca Hunkeler Committed by Commit Bot

[Autofill Assistant] Improve logic to hide chips on keyboard shown

Currently, if an element check is being performed the chips will not
be correctly hidden when the keyboard is shown. This happens because
after the periodic element check we reset the visibity to the default
value (true). This change reworks how we hide the chips and fixes that
problem.

Bug: b/171179162
Change-Id: Icb3149fb31449d2ad2dedc114b710f195275ff57
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2484696
Commit-Queue: Luca Hunkeler <hluca@google.com>
Reviewed-by: default avatarSandro Maggi <sandromaggi@google.com>
Cr-Commit-Position: refs/heads/master@{#818457}
parent 49a91671
...@@ -15,7 +15,6 @@ import org.chromium.base.task.PostTask; ...@@ -15,7 +15,6 @@ import org.chromium.base.task.PostTask;
import org.chromium.chrome.autofill_assistant.R; import org.chromium.chrome.autofill_assistant.R;
import org.chromium.chrome.browser.ActivityTabProvider; import org.chromium.chrome.browser.ActivityTabProvider;
import org.chromium.chrome.browser.app.ChromeActivity; import org.chromium.chrome.browser.app.ChromeActivity;
import org.chromium.chrome.browser.autofill_assistant.carousel.AssistantCarouselModel;
import org.chromium.chrome.browser.autofill_assistant.carousel.AssistantChip; import org.chromium.chrome.browser.autofill_assistant.carousel.AssistantChip;
import org.chromium.chrome.browser.autofill_assistant.carousel.AssistantChip.Type; import org.chromium.chrome.browser.autofill_assistant.carousel.AssistantChip.Type;
import org.chromium.chrome.browser.autofill_assistant.metrics.DropOutReason; import org.chromium.chrome.browser.autofill_assistant.metrics.DropOutReason;
...@@ -339,9 +338,9 @@ public class AutofillAssistantUiController { ...@@ -339,9 +338,9 @@ public class AutofillAssistantUiController {
*/ */
@CalledByNative @CalledByNative
private AssistantChip createActionButton(int icon, String text, int actionIndex, private AssistantChip createActionButton(int icon, String text, int actionIndex,
boolean disabled, boolean sticky, String identifier) { boolean disabled, boolean sticky, boolean visible) {
return new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, icon, text, disabled, sticky, return new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, icon, text, disabled, sticky,
identifier, () -> safeNativeOnUserActionSelected(actionIndex)); visible, () -> safeNativeOnUserActionSelected(actionIndex));
} }
/** /**
...@@ -349,8 +348,8 @@ public class AutofillAssistantUiController { ...@@ -349,8 +348,8 @@ public class AutofillAssistantUiController {
*/ */
@CalledByNative @CalledByNative
private AssistantChip createHighlightedActionButton(int icon, String text, int actionIndex, private AssistantChip createHighlightedActionButton(int icon, String text, int actionIndex,
boolean disabled, boolean sticky, String identifier) { boolean disabled, boolean sticky, boolean visible) {
return new AssistantChip(Type.BUTTON_FILLED_BLUE, icon, text, disabled, sticky, identifier, return new AssistantChip(Type.BUTTON_FILLED_BLUE, icon, text, disabled, sticky, visible,
() -> safeNativeOnUserActionSelected(actionIndex)); () -> safeNativeOnUserActionSelected(actionIndex));
} }
...@@ -361,9 +360,9 @@ public class AutofillAssistantUiController { ...@@ -361,9 +360,9 @@ public class AutofillAssistantUiController {
*/ */
@CalledByNative @CalledByNative
private AssistantChip createCancelButton(int icon, String text, int actionIndex, private AssistantChip createCancelButton(int icon, String text, int actionIndex,
boolean disabled, boolean sticky, String identifier) { boolean disabled, boolean sticky, boolean visible) {
return new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, icon, text, disabled, sticky, return new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, icon, text, disabled, sticky,
identifier, () -> safeNativeOnCancelButtonClicked(actionIndex)); visible, () -> safeNativeOnCancelButtonClicked(actionIndex));
} }
/** /**
...@@ -371,9 +370,9 @@ public class AutofillAssistantUiController { ...@@ -371,9 +370,9 @@ public class AutofillAssistantUiController {
*/ */
@CalledByNative @CalledByNative
private AssistantChip createCloseButton( private AssistantChip createCloseButton(
int icon, String text, boolean disabled, boolean sticky, String identifier) { int icon, String text, boolean disabled, boolean sticky, boolean visible) {
return new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, icon, text, disabled, sticky, return new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, icon, text, disabled, sticky,
identifier, this::safeNativeOnCloseButtonClicked); visible, this::safeNativeOnCloseButtonClicked);
} }
@CalledByNative @CalledByNative
...@@ -393,24 +392,6 @@ public class AutofillAssistantUiController { ...@@ -393,24 +392,6 @@ public class AutofillAssistantUiController {
getModel().getActionsModel().setDisableChangeAnimations(disable); getModel().getActionsModel().setDisableChangeAnimations(disable);
} }
@CalledByNative
private void setAllChipsVisibleExcept(String identifier, boolean visible) {
AssistantCarouselModel model = getModel().getActionsModel();
List<AssistantChip> chips = model.get(AssistantCarouselModel.CHIPS);
// Copy the list and modify the copy. Modifying the actual list in-place will not fire the
// relevant change notifications. TODO(b/144075373): Refactor to avoid this deep copy,
// preferably by moving this to native.
List<AssistantChip> newChips = new ArrayList<>();
for (int i = 0; i < chips.size(); ++i) {
AssistantChip newChip = new AssistantChip(chips.get(i));
newChips.add(newChip);
if (!chips.get(i).getIdentifier().equals(identifier)) {
newChip.setVisible(visible);
}
}
model.setChips(newChips);
}
@CalledByNative @CalledByNative
private void setViewportMode(@AssistantViewportMode int mode) { private void setViewportMode(@AssistantViewportMode int mode) {
mCoordinator.getBottomBarCoordinator().setViewportMode(mode); mCoordinator.getBottomBarCoordinator().setViewportMode(mode);
......
...@@ -67,34 +67,20 @@ public class AssistantChip { ...@@ -67,34 +67,20 @@ public class AssistantChip {
*/ */
private final boolean mSticky; private final boolean mSticky;
private final String mIdentifier;
/** The callback that will be triggered when this chip is clicked. */ /** The callback that will be triggered when this chip is clicked. */
private final Runnable mSelectedListener; private final Runnable mSelectedListener;
public AssistantChip(@Type int type, @Icon int icon, String text, boolean disabled, public AssistantChip(@Type int type, @Icon int icon, String text, boolean disabled,
boolean sticky, String identifier, Runnable selectedListener) { boolean sticky, boolean visible, Runnable selectedListener) {
mType = type; mType = type;
mIcon = icon; mIcon = icon;
mText = text; mText = text;
mDisabled = disabled; mDisabled = disabled;
mVisible = true;
mSticky = sticky; mSticky = sticky;
mIdentifier = identifier; mVisible = visible;
mSelectedListener = selectedListener; mSelectedListener = selectedListener;
} }
public AssistantChip(AssistantChip other) {
mType = other.mType;
mIcon = other.mIcon;
mText = other.mText;
mDisabled = other.mDisabled;
mVisible = other.mVisible;
mSticky = other.mSticky;
mIdentifier = other.mIdentifier;
mSelectedListener = other.mSelectedListener;
}
public int getType() { public int getType() {
return mType; return mType;
} }
...@@ -127,10 +113,6 @@ public class AssistantChip { ...@@ -127,10 +113,6 @@ public class AssistantChip {
return mSticky; return mSticky;
} }
public String getIdentifier() {
return mIdentifier;
}
public Runnable getSelectedListener() { public Runnable getSelectedListener() {
return mSelectedListener; return mSelectedListener;
} }
...@@ -144,7 +126,6 @@ public class AssistantChip { ...@@ -144,7 +126,6 @@ public class AssistantChip {
AssistantChip that = (AssistantChip) other; AssistantChip that = (AssistantChip) other;
return this.getType() == that.getType() && this.getText().equals(that.getText()) return this.getType() == that.getType() && this.getText().equals(that.getText())
&& this.getIcon() == that.getIcon() && this.isSticky() == that.isSticky() && this.getIcon() == that.getIcon() && this.isSticky() == that.isSticky()
&& this.getIdentifier().equals(that.getIdentifier())
&& this.isDisabled() == that.isDisabled() && this.isVisible() == that.isVisible(); && this.isDisabled() == that.isDisabled() && this.isVisible() == that.isVisible();
} }
} }
...@@ -96,7 +96,7 @@ public class AutofillAssistantActionsCarouselUiTest { ...@@ -96,7 +96,7 @@ public class AutofillAssistantActionsCarouselUiTest {
-> model.set(AssistantCarouselModel.CHIPS, -> model.set(AssistantCarouselModel.CHIPS,
Collections.singletonList(new AssistantChip( Collections.singletonList(new AssistantChip(
AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE, AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE,
"Test", false, true, "", null)))); "Test", false, true, true, null))));
// Chip was created and is displayed on the screen. // Chip was created and is displayed on the screen.
onView(is(coordinator.getView())) onView(is(coordinator.getView()))
...@@ -117,10 +117,10 @@ public class AutofillAssistantActionsCarouselUiTest { ...@@ -117,10 +117,10 @@ public class AutofillAssistantActionsCarouselUiTest {
List<AssistantChip> chips = new ArrayList<>(); List<AssistantChip> chips = new ArrayList<>();
for (int i = 0; i < numChips; i++) { for (int i = 0; i < numChips; i++) {
chips.add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE, chips.add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE,
"T" + i, false, false, "", null)); "T" + i, false, false, true, null));
} }
chips.add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE, chips.add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE,
"X", false, true, "", null)); "X", false, true, true, null));
TestThreadUtils.runOnUiThreadBlocking(() -> model.set(AssistantCarouselModel.CHIPS, chips)); TestThreadUtils.runOnUiThreadBlocking(() -> model.set(AssistantCarouselModel.CHIPS, chips));
...@@ -145,10 +145,10 @@ public class AutofillAssistantActionsCarouselUiTest { ...@@ -145,10 +145,10 @@ public class AutofillAssistantActionsCarouselUiTest {
List<AssistantChip> chips = new ArrayList<>(); List<AssistantChip> chips = new ArrayList<>();
for (int i = 0; i < numChips; i++) { for (int i = 0; i < numChips; i++) {
chips.add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE, chips.add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE,
"Test" + i, false, false, "", null)); "Test" + i, false, false, true, null));
} }
chips.add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE, chips.add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE,
"Cancel", false, true, "", null)); "Cancel", false, true, true, null));
TestThreadUtils.runOnUiThreadBlocking(() -> model.set(AssistantCarouselModel.CHIPS, chips)); TestThreadUtils.runOnUiThreadBlocking(() -> model.set(AssistantCarouselModel.CHIPS, chips));
// Cancel chip is initially displayed to the user. // Cancel chip is initially displayed to the user.
...@@ -174,9 +174,9 @@ public class AutofillAssistantActionsCarouselUiTest { ...@@ -174,9 +174,9 @@ public class AutofillAssistantActionsCarouselUiTest {
List<AssistantChip> chips = new ArrayList<>(); List<AssistantChip> chips = new ArrayList<>();
chips.add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE, chips.add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE,
"Test 2", false, false, "", null)); "Test 2", false, false, true, null));
chips.add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE, chips.add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE,
"Cancel", false, true, "", null)); "Cancel", false, true, true, null));
TestThreadUtils.runOnUiThreadBlocking(() -> model.set(AssistantCarouselModel.CHIPS, chips)); TestThreadUtils.runOnUiThreadBlocking(() -> model.set(AssistantCarouselModel.CHIPS, chips));
onView(withText("Cancel")).check(matches(isDisplayed())); onView(withText("Cancel")).check(matches(isDisplayed()));
onView(withText("Test 2")).check(matches(isDisplayed())); onView(withText("Test 2")).check(matches(isDisplayed()));
...@@ -185,7 +185,7 @@ public class AutofillAssistantActionsCarouselUiTest { ...@@ -185,7 +185,7 @@ public class AutofillAssistantActionsCarouselUiTest {
List<AssistantChip> newChips = new ArrayList<>(); List<AssistantChip> newChips = new ArrayList<>();
newChips.add(chips.get(0)); newChips.add(chips.get(0));
newChips.add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE, newChips.add(new AssistantChip(AssistantChip.Type.BUTTON_HAIRLINE, AssistantChip.Icon.NONE,
"Test 1", false, false, "", null)); "Test 1", false, false, true, null));
newChips.add(chips.get(1)); newChips.add(chips.get(1));
TestThreadUtils.runOnUiThreadBlocking( TestThreadUtils.runOnUiThreadBlocking(
() -> model.set(AssistantCarouselModel.CHIPS, newChips)); () -> model.set(AssistantCarouselModel.CHIPS, newChips));
......
...@@ -206,8 +206,9 @@ public class AutofillAssistantHeaderUiTest { ...@@ -206,8 +206,9 @@ public class AutofillAssistantHeaderUiTest {
AssistantHeaderCoordinator coordinator = createCoordinator(model); AssistantHeaderCoordinator coordinator = createCoordinator(model);
String chipText = "Hello World"; String chipText = "Hello World";
AssistantChip chip = new AssistantChip(AssistantChip.Type.BUTTON_FILLED_BLUE, Icon.DONE, AssistantChip chip =
chipText, /* disabled= */ false, /* sticky= */ false, "", () -> {}); new AssistantChip(AssistantChip.Type.BUTTON_FILLED_BLUE, Icon.DONE, chipText,
/* disabled= */ false, /* sticky= */ false, /* visible= */ true, () -> {});
// Set the header chip without displaying it. // Set the header chip without displaying it.
List<AssistantChip> chips = new ArrayList<>(); List<AssistantChip> chips = new ArrayList<>();
......
...@@ -6,11 +6,14 @@ package org.chromium.chrome.browser.autofill_assistant; ...@@ -6,11 +6,14 @@ package org.chromium.chrome.browser.autofill_assistant;
import static androidx.test.espresso.Espresso.onView; import static androidx.test.espresso.Espresso.onView;
import static androidx.test.espresso.action.ViewActions.click; import static androidx.test.espresso.action.ViewActions.click;
import static androidx.test.espresso.assertion.ViewAssertions.matches;
import static androidx.test.espresso.matcher.ViewMatchers.isCompletelyDisplayed; import static androidx.test.espresso.matcher.ViewMatchers.isCompletelyDisplayed;
import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed;
import static androidx.test.espresso.matcher.ViewMatchers.withText; import static androidx.test.espresso.matcher.ViewMatchers.withText;
import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiTestUtil.getElementValue; import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiTestUtil.getElementValue;
import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiTestUtil.tapElement; import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiTestUtil.tapElement;
...@@ -36,6 +39,7 @@ import org.chromium.chrome.browser.autofill_assistant.proto.ClickProto; ...@@ -36,6 +39,7 @@ import org.chromium.chrome.browser.autofill_assistant.proto.ClickProto;
import org.chromium.chrome.browser.autofill_assistant.proto.ClickType; import org.chromium.chrome.browser.autofill_assistant.proto.ClickType;
import org.chromium.chrome.browser.autofill_assistant.proto.ElementAreaProto; import org.chromium.chrome.browser.autofill_assistant.proto.ElementAreaProto;
import org.chromium.chrome.browser.autofill_assistant.proto.ElementAreaProto.Rectangle; import org.chromium.chrome.browser.autofill_assistant.proto.ElementAreaProto.Rectangle;
import org.chromium.chrome.browser.autofill_assistant.proto.ElementConditionProto;
import org.chromium.chrome.browser.autofill_assistant.proto.FocusElementProto; import org.chromium.chrome.browser.autofill_assistant.proto.FocusElementProto;
import org.chromium.chrome.browser.autofill_assistant.proto.KeyboardValueFillStrategy; import org.chromium.chrome.browser.autofill_assistant.proto.KeyboardValueFillStrategy;
import org.chromium.chrome.browser.autofill_assistant.proto.PromptProto; import org.chromium.chrome.browser.autofill_assistant.proto.PromptProto;
...@@ -304,4 +308,68 @@ public class AutofillAssistantKeyboardIntegrationTest { ...@@ -304,4 +308,68 @@ public class AutofillAssistantKeyboardIntegrationTest {
tapElement(mTestRule, "iframe", "name"); tapElement(mTestRule, "iframe", "name");
waitUntilKeyboardMatchesCondition(mTestRule, /* isShowing= */ true); waitUntilKeyboardMatchesCondition(mTestRule, /* isShowing= */ true);
} }
@Test
@MediumTest
public void hideChipsWhileKeyboardShowing() throws Exception {
SelectorProto element =
(SelectorProto) SelectorProto.newBuilder()
.addFilters(
SelectorProto.Filter.newBuilder().setCssSelector("#profile_name"))
.build();
ArrayList<ActionProto> list = new ArrayList<>();
list.add(
(ActionProto) ActionProto.newBuilder()
.setFocusElement(FocusElementProto.newBuilder()
.setElement(element)
.setTouchableElementArea(
ElementAreaProto.newBuilder().addTouchable(
Rectangle.newBuilder().addElements(
element))))
.build());
list.add((ActionProto) ActionProto.newBuilder()
.setPrompt(
PromptProto.newBuilder()
.setMessage("Highlighted")
.addChoices(
Choice.newBuilder()
.setChip(ChipProto.newBuilder().setText(
"Done"))
.setShowOnlyWhen(
ElementConditionProto.newBuilder()
.setMatch(element)))
.addChoices(Choice.newBuilder().setChip(
ChipProto.newBuilder()
.setType(ChipType.CANCEL_ACTION)
.setText("Cancel"))))
.build());
AutofillAssistantTestScript script = new AutofillAssistantTestScript(
(SupportedScriptProto) SupportedScriptProto.newBuilder()
.setPath(TEST_PAGE)
.setPresentation(PresentationProto.newBuilder().setAutostart(true).setChip(
ChipProto.newBuilder().setText("Done")))
.build(),
list);
runAutofillAssistant(script);
waitUntilViewMatchesCondition(withText("Highlighted"), isCompletelyDisplayed());
waitUntilViewMatchesCondition(withText("Done"), isDisplayed());
onView(withText("Cancel")).check(matches(isDisplayed()));
tapElement(mTestRule, "profile_name");
waitUntilKeyboardMatchesCondition(mTestRule, /* isShowing= */ true);
waitUntilViewMatchesCondition(withText("Done"), not(isDisplayed()));
// Chips of type CANCEL should stay visible.
onView(withText("Cancel")).check(matches(isDisplayed()));
// Clicking on a cancel chip while the keyboard is showing hides the keyboard instead of
// closing Autofill Assistant.
onView(withText("Cancel")).perform(click());
waitUntilKeyboardMatchesCondition(mTestRule, /* isShowing= */ false);
waitUntilViewMatchesCondition(withText("Done"), isDisplayed());
onView(withText("Cancel")).check(matches(isDisplayed()));
}
} }
...@@ -233,13 +233,15 @@ public class AutofillAssistantUiTest { ...@@ -233,13 +233,15 @@ public class AutofillAssistantUiTest {
private void testChips(InOrder inOrder, AssistantCarouselModel carouselModel, private void testChips(InOrder inOrder, AssistantCarouselModel carouselModel,
AssistantActionsCarouselCoordinator carouselCoordinator) { AssistantActionsCarouselCoordinator carouselCoordinator) {
List<AssistantChip> chips = Arrays.asList( List<AssistantChip> chips =
new AssistantChip(AssistantChip.Type.CHIP_ASSISTIVE, AssistantChip.Icon.NONE, Arrays.asList(new AssistantChip(AssistantChip.Type.CHIP_ASSISTIVE,
"chip 0", AssistantChip.Icon.NONE, "chip 0",
/* disabled= */ false, /* sticky= */ false, "", () -> {/* do nothing */}), /* disabled= */ false, /* sticky= */ false,
new AssistantChip(AssistantChip.Type.CHIP_ASSISTIVE, AssistantChip.Icon.NONE, /* visible= */ true, () -> {/* do nothing */}),
"chip 1", new AssistantChip(AssistantChip.Type.CHIP_ASSISTIVE,
/* disabled= */ false, /* sticky= */ false, "", mRunnableMock)); AssistantChip.Icon.NONE, "chip 1",
/* disabled= */ false, /* sticky= */ false, /* visible= */ true,
mRunnableMock));
TestThreadUtils.runOnUiThreadBlocking(() -> carouselModel.setChips(chips)); TestThreadUtils.runOnUiThreadBlocking(() -> carouselModel.setChips(chips));
RecyclerView chipsViewContainer = carouselCoordinator.getView(); RecyclerView chipsViewContainer = carouselCoordinator.getView();
Assert.assertEquals(2, chipsViewContainer.getAdapter().getItemCount()); Assert.assertEquals(2, chipsViewContainer.getAdapter().getItemCount());
......
...@@ -68,9 +68,6 @@ using base::android::JavaRef; ...@@ -68,9 +68,6 @@ using base::android::JavaRef;
namespace autofill_assistant { namespace autofill_assistant {
namespace { namespace {
static const char* const kCancelChipIdentifier = "CANCEL_CHIP_ID";
std::vector<float> ToFloatVector(const std::vector<RectF>& areas) { std::vector<float> ToFloatVector(const std::vector<RectF>& areas) {
std::vector<float> flattened; std::vector<float> flattened;
for (const auto& rect : areas) { for (const auto& rect : areas) {
...@@ -763,24 +760,18 @@ void UiControllerAndroid::UpdateActions( ...@@ -763,24 +760,18 @@ void UiControllerAndroid::UpdateActions(
break; break;
case HIGHLIGHTED_ACTION: case HIGHLIGHTED_ACTION:
// Here and below, we set the identifier to the empty string so that we
// can hide all the chips except for the cancel chip when the keyboard
// is showing.
// TODO(b/149543425): Find a better way to do this.
jchip = jchip =
Java_AutofillAssistantUiController_createHighlightedActionButton( Java_AutofillAssistantUiController_createHighlightedActionButton(
env, java_object_, chip.icon, env, java_object_, chip.icon,
base::android::ConvertUTF8ToJavaString(env, chip.text), i, base::android::ConvertUTF8ToJavaString(env, chip.text), i,
!action.enabled(), chip.sticky, !action.enabled(), chip.sticky, chip.visible);
base::android::ConvertUTF8ToJavaString(env, ""));
break; break;
case NORMAL_ACTION: case NORMAL_ACTION:
jchip = Java_AutofillAssistantUiController_createActionButton( jchip = Java_AutofillAssistantUiController_createActionButton(
env, java_object_, chip.icon, env, java_object_, chip.icon,
base::android::ConvertUTF8ToJavaString(env, chip.text), i, base::android::ConvertUTF8ToJavaString(env, chip.text), i,
!action.enabled(), chip.sticky, !action.enabled(), chip.sticky, chip.visible);
base::android::ConvertUTF8ToJavaString(env, ""));
break; break;
case CANCEL_ACTION: case CANCEL_ACTION:
...@@ -789,8 +780,7 @@ void UiControllerAndroid::UpdateActions( ...@@ -789,8 +780,7 @@ void UiControllerAndroid::UpdateActions(
jchip = Java_AutofillAssistantUiController_createCancelButton( jchip = Java_AutofillAssistantUiController_createCancelButton(
env, java_object_, chip.icon, env, java_object_, chip.icon,
base::android::ConvertUTF8ToJavaString(env, chip.text), i, base::android::ConvertUTF8ToJavaString(env, chip.text), i,
!action.enabled(), chip.sticky, !action.enabled(), chip.sticky, chip.visible);
base::android::ConvertUTF8ToJavaString(env, kCancelChipIdentifier));
has_close_or_cancel = true; has_close_or_cancel = true;
break; break;
...@@ -798,8 +788,7 @@ void UiControllerAndroid::UpdateActions( ...@@ -798,8 +788,7 @@ void UiControllerAndroid::UpdateActions(
jchip = Java_AutofillAssistantUiController_createActionButton( jchip = Java_AutofillAssistantUiController_createActionButton(
env, java_object_, chip.icon, env, java_object_, chip.icon,
base::android::ConvertUTF8ToJavaString(env, chip.text), i, base::android::ConvertUTF8ToJavaString(env, chip.text), i,
!action.enabled(), chip.sticky, !action.enabled(), chip.sticky, chip.visible);
base::android::ConvertUTF8ToJavaString(env, ""));
has_close_or_cancel = true; has_close_or_cancel = true;
break; break;
...@@ -808,8 +797,7 @@ void UiControllerAndroid::UpdateActions( ...@@ -808,8 +797,7 @@ void UiControllerAndroid::UpdateActions(
Java_AutofillAssistantUiController_createHighlightedActionButton( Java_AutofillAssistantUiController_createHighlightedActionButton(
env, java_object_, chip.icon, env, java_object_, chip.icon,
base::android::ConvertUTF8ToJavaString(env, chip.text), i, base::android::ConvertUTF8ToJavaString(env, chip.text), i,
!action.enabled(), chip.sticky, !action.enabled(), chip.sticky, chip.visible);
base::android::ConvertUTF8ToJavaString(env, ""));
has_close_or_cancel = true; has_close_or_cancel = true;
break; break;
} }
...@@ -828,14 +816,12 @@ void UiControllerAndroid::UpdateActions( ...@@ -828,14 +816,12 @@ void UiControllerAndroid::UpdateActions(
jcancel_chip = Java_AutofillAssistantUiController_createCloseButton( jcancel_chip = Java_AutofillAssistantUiController_createCloseButton(
env, java_object_, ICON_CLEAR, env, java_object_, ICON_CLEAR,
base::android::ConvertUTF8ToJavaString(env, ""), base::android::ConvertUTF8ToJavaString(env, ""),
/* disabled= */ false, /* sticky= */ true, /* disabled= */ false, /* sticky= */ true, /* visible=*/true);
base::android::ConvertUTF8ToJavaString(env, ""));
} else if (ui_delegate_->GetState() != AutofillAssistantState::INACTIVE) { } else if (ui_delegate_->GetState() != AutofillAssistantState::INACTIVE) {
jcancel_chip = Java_AutofillAssistantUiController_createCancelButton( jcancel_chip = Java_AutofillAssistantUiController_createCancelButton(
env, java_object_, ICON_CLEAR, env, java_object_, ICON_CLEAR,
base::android::ConvertUTF8ToJavaString(env, ""), -1, base::android::ConvertUTF8ToJavaString(env, ""), -1,
/* disabled= */ false, /* sticky= */ true, /* disabled= */ false, /* sticky= */ true, /* visible=*/true);
base::android::ConvertUTF8ToJavaString(env, kCancelChipIdentifier));
} }
if (jcancel_chip) { if (jcancel_chip) {
Java_AutofillAssistantUiController_appendChipToList(env, jchips, Java_AutofillAssistantUiController_appendChipToList(env, jchips,
...@@ -889,13 +875,8 @@ void UiControllerAndroid::OnKeyboardVisibilityChanged( ...@@ -889,13 +875,8 @@ void UiControllerAndroid::OnKeyboardVisibilityChanged(
JNIEnv* env, JNIEnv* env,
const base::android::JavaParamRef<jobject>& jcaller, const base::android::JavaParamRef<jobject>& jcaller,
jboolean visible) { jboolean visible) {
// Hide all chips except cancel while the keyboard is shown, to prevent users if (ui_delegate_)
// from accidentally tapping chips while using the keyboard. ui_delegate_->OnKeyboardVisibilityChanged(visible);
// TODO(b/149543425): Find a better way to do this.
Java_AutofillAssistantUiController_setAllChipsVisibleExcept(
env, java_object_,
base::android::ConvertUTF8ToJavaString(env, kCancelChipIdentifier),
!visible);
} }
bool UiControllerAndroid::OnBackButtonClicked() { bool UiControllerAndroid::OnBackButtonClicked() {
......
...@@ -34,6 +34,9 @@ struct Chip { ...@@ -34,6 +34,9 @@ struct Chip {
// Whether this chip is sticky. A sticky chip will be a candidate to be // Whether this chip is sticky. A sticky chip will be a candidate to be
// displayed in the header if the peek mode of the sheet is HANDLE_HEADER. // displayed in the header if the peek mode of the sheet is HANDLE_HEADER.
bool sticky = false; bool sticky = false;
// Whether this chip should be displayed in the carousel.
bool visible = true;
}; };
// Guarantees that the Chip.type of all chips is set to a sensible value. // Guarantees that the Chip.type of all chips is set to a sensible value.
......
...@@ -380,6 +380,19 @@ void Controller::SetUserActions( ...@@ -380,6 +380,19 @@ void Controller::SetUserActions(
SetDefaultChipType(user_actions.get()); SetDefaultChipType(user_actions.get());
} }
user_actions_ = std::move(user_actions); user_actions_ = std::move(user_actions);
SetVisibilityAndUpdateUserActions();
}
void Controller::SetVisibilityAndUpdateUserActions() {
// All non-cancel chips should be hidden while the keyboard is showing.
if (user_actions_) {
for (UserAction& user_action : *user_actions_) {
if (user_action.chip().type != CANCEL_ACTION) {
user_action.chip().visible = !is_keyboard_showing_;
}
}
}
for (ControllerObserver& observer : observers_) { for (ControllerObserver& observer : observers_) {
observer.OnUserActionsChanged(GetUserActions()); observer.OnUserActionsChanged(GetUserActions());
} }
...@@ -2006,6 +2019,11 @@ bool Controller::IsRunningLiteScript() const { ...@@ -2006,6 +2019,11 @@ bool Controller::IsRunningLiteScript() const {
return service_ ? service_->IsLiteService() : false; return service_ ? service_->IsLiteService() : false;
} }
void Controller::OnKeyboardVisibilityChanged(bool visible) {
is_keyboard_showing_ = visible;
SetVisibilityAndUpdateUserActions();
}
ElementArea* Controller::touchable_element_area() { ElementArea* Controller::touchable_element_area() {
if (!touchable_element_area_) { if (!touchable_element_area_) {
touchable_element_area_ = std::make_unique<ElementArea>(this); touchable_element_area_ = std::make_unique<ElementArea>(this);
......
...@@ -251,6 +251,7 @@ class Controller : public ScriptExecutorDelegate, ...@@ -251,6 +251,7 @@ class Controller : public ScriptExecutorDelegate,
bool ShouldShowOverlay() const override; bool ShouldShowOverlay() const override;
void ShutdownIfNecessary() override; void ShutdownIfNecessary() override;
bool IsRunningLiteScript() const override; bool IsRunningLiteScript() const override;
void OnKeyboardVisibilityChanged(bool visible) override;
private: private:
friend ControllerTest; friend ControllerTest;
...@@ -354,6 +355,8 @@ class Controller : public ScriptExecutorDelegate, ...@@ -354,6 +355,8 @@ class Controller : public ScriptExecutorDelegate,
bool StateNeedsUI(AutofillAssistantState state); bool StateNeedsUI(AutofillAssistantState state);
void SetVisibilityAndUpdateUserActions();
ClientSettings settings_; ClientSettings settings_;
Client* const client_; Client* const client_;
const base::TickClock* const tick_clock_; const base::TickClock* const tick_clock_;
...@@ -521,6 +524,7 @@ class Controller : public ScriptExecutorDelegate, ...@@ -521,6 +524,7 @@ class Controller : public ScriptExecutorDelegate,
bool expand_sheet_for_prompt_action_ = true; bool expand_sheet_for_prompt_action_ = true;
std::vector<std::string> browse_domains_allowlist_; std::vector<std::string> browse_domains_allowlist_;
bool browse_mode_invisible_ = false; bool browse_mode_invisible_ = false;
bool is_keyboard_showing_ = false;
// Only set during a ShowGenericUiAction. // Only set during a ShowGenericUiAction.
std::unique_ptr<GenericUserInterfaceProto> generic_user_interface_; std::unique_ptr<GenericUserInterfaceProto> generic_user_interface_;
......
...@@ -263,6 +263,9 @@ class UiDelegate { ...@@ -263,6 +263,9 @@ class UiDelegate {
// Returns whether the UI delegate is currently running a lite script or not. // Returns whether the UI delegate is currently running a lite script or not.
virtual bool IsRunningLiteScript() const = 0; virtual bool IsRunningLiteScript() const = 0;
// Called when the visibility of the keyboard has changed.
virtual void OnKeyboardVisibilityChanged(bool visible) = 0;
protected: protected:
UiDelegate() = default; UiDelegate() = default;
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment