Skip to content

GitLab

T
tangled
Commit b3f39033 authored by Alex Ilin's avatar Alex Ilin Committed by Commit Bot
Browse files
Options

Revert "[Autofill Assistant] Hotfix for radio buttons in FormAction." 

This reverts commit 8698d875.

Reason for revert: causes AutofillAssistantFormActionTest#testFormAction fail on Lollipop Phone Tester https://ci.chromium.org/p/chromium/builders/ci/Lollipop%20Phone%20Tester/24483

Original change's description:
> [Autofill Assistant] Hotfix for radio buttons in FormAction.
> 
> Without this CL, UI and model can go out-of-sync when tapping the radio
> button directly (see screenshots in linked bug). This is a hotfix for
> this issue. A proper fix requires a refactoring of the FormAction to
> make the selection state part of its model (same as was done for the
> T&C selection state in the CollectUserDataModel).
> 
> Bug: b/150201921
> Change-Id: Ifa0a2839c532a1862358bc68e6d0537dee93b49c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2083489
> Commit-Queue: Clemens Arbesser <arbesser@google.com>
> Reviewed-by: Mathias Carlen <mcarlen@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#746539}

TBR=mcarlen@chromium.org,arbesser@google.com,marianfe@google.com

Change-Id: Ie495ee0ae938de693dd9b5667086dca05b5d9d32
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b/150201921
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2087571Reviewed-by: default avatarAlex Ilin <alexilin@chromium.org>
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746707}
parent b62ae460
Hide whitespace changes
Inline Side-by-side
Showing with 12 additions and 75 deletions
chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/form/AssistantFormSelectionInput.java
View file @ b3f39033
......@@ -15,7 +15,6 @@ import org.chromium.chrome.autofill_assistant.R;
import org.chromium.chrome.browser.autofill_assistant.AssistantTextUtils;
import org.chromium.chrome.browser.autofill_assistant.user_data.AssistantChoiceList;
import java.util.ArrayList;
import java.util.List;
/** A form input that allows to choose between multiple choices. */
......@@ -29,7 +28,6 @@ class AssistantFormSelectionInput extends AssistantFormInput {
private final List<AssistantFormSelectionChoice> mChoices;
private final boolean mAllowMultipleChoices;
private final Delegate mDelegate;
private final List<View> mChoiceViews = new ArrayList<>();
public AssistantFormSelectionInput(String label, List<AssistantFormSelectionChoice> choices,
boolean allowMultipleChoices, Delegate delegate) {
......@@ -58,7 +56,6 @@ class AssistantFormSelectionInput extends AssistantFormInput {
ViewGroup checkboxList = root.findViewById(R.id.checkbox_list);
AssistantChoiceList radiobuttonList = root.findViewById(R.id.radiobutton_list);
radiobuttonList.setAllowMultipleChoices(false);
for (int i = 0; i < mChoices.size(); i++) {
AssistantFormSelectionChoice choice = mChoices.get(i);
......@@ -82,24 +79,10 @@ class AssistantFormSelectionInput extends AssistantFormInput {
radiobuttonList.addItem(choiceView, /* hasEditButton= */ false,
(isChecked)
-> {
mDelegate.onChoiceSelectionChanged(index, isChecked);
// Workaround for radio buttons in FormAction: de-select all other
// items. This is needed because the current selection state is not part
// of AssistantFormModel (but it should be). TODO(b/150201921).
if (isChecked) {
for (View view : mChoiceViews) {
if (view == choiceView) {
continue;
}
radiobuttonList.setChecked(view, false);
}
}
},
-> mDelegate.onChoiceSelectionChanged(index, isChecked),
/* itemEditedListener= */ null);
radiobuttonList.setChecked(choiceView, choice.isInitiallySelected());
}
mChoiceViews.add(choiceView);
TextView choiceLabel = choiceView.findViewById(R.id.label);
TextView descriptionLine1 = choiceView.findViewById(R.id.description_line_1);
......
chrome/android/features/autofill_assistant/javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantFormActionTest.java
View file @ b3f39033
......@@ -13,17 +13,13 @@ import static android.support.test.espresso.intent.Intents.intending;
import static android.support.test.espresso.intent.matcher.IntentMatchers.anyIntent;
import static android.support.test.espresso.intent.matcher.IntentMatchers.hasAction;
import static android.support.test.espresso.intent.matcher.IntentMatchers.hasData;
import static android.support.test.espresso.matcher.ViewMatchers.Visibility.VISIBLE;
import static android.support.test.espresso.matcher.ViewMatchers.assertThat;
import static android.support.test.espresso.matcher.ViewMatchers.hasDescendant;
import static android.support.test.espresso.matcher.ViewMatchers.hasSibling;
import static android.support.test.espresso.matcher.ViewMatchers.hasTextColor;
import static android.support.test.espresso.matcher.ViewMatchers.isChecked;
import static android.support.test.espresso.matcher.ViewMatchers.isCompletelyDisplayed;
import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed;
import static android.support.test.espresso.matcher.ViewMatchers.isEnabled;
import static android.support.test.espresso.matcher.ViewMatchers.withClassName;
import static android.support.test.espresso.matcher.ViewMatchers.withEffectiveVisibility;
import static android.support.test.espresso.matcher.ViewMatchers.withId;
import static android.support.test.espresso.matcher.ViewMatchers.withText;
......@@ -34,7 +30,6 @@ import static org.hamcrest.Matchers.not;
import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiTestUtil.hasTintColor;
import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiTestUtil.startAutofillAssistant;
import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiTestUtil.waitUntilViewMatchesCondition;
import static org.chromium.chrome.browser.autofill_assistant.AutofillAssistantUiTestUtil.withParentIndex;
import android.app.Activity;
import android.app.Instrumentation.ActivityResult;
......@@ -43,7 +38,6 @@ import android.net.Uri;
import android.support.test.InstrumentationRegistry;
import android.support.test.espresso.intent.Intents;
import android.support.test.filters.MediumTest;
import android.widget.RadioButton;
import org.junit.Before;
import org.junit.Rule;
......@@ -134,17 +128,13 @@ public class AutofillAssistantFormActionTest {
.setExpandText("Expand")))
.addInputs(FormInputProto.newBuilder().setSelection(
SelectionInputProto.newBuilder()
.addChoices(SelectionInputProto.Choice.newBuilder()
.setLabel("Choice 1")
.setDescriptionLine1("$10.00 option")
.setDescriptionLine2(
"<link1>Details</link1>"))
.addChoices(SelectionInputProto.Choice.newBuilder()
.setLabel("Choice 2")
.setDescriptionLine1("$20.00 option")
.setDescriptionLine2(
"<link1>Details</link1>"))
.setAllowMultiple(false)))
.addChoices(
SelectionInputProto.Choice.newBuilder()
.setLabel("Choice 1")
.setDescriptionLine1("$10.00 per choice")
.setDescriptionLine2(
"<link1>Details</link1>"))
.setAllowMultiple(true)))
.addInputs(FormInputProto.newBuilder().setCounter(
CounterInputProto.newBuilder().addCounters(
CounterInputProto.Counter.newBuilder()
......@@ -204,25 +194,10 @@ public class AutofillAssistantFormActionTest {
hasSibling(hasDescendant(withText("Counter 3")))))
.perform(click());
// Click on Choice 1, then Choice 2, then back to Choice 1.
onView(allOf(withClassName(is(RadioButton.class.getName())), withParentIndex(0),
withEffectiveVisibility(VISIBLE)))
// Click on Choice 1, toggle to 'checked'.
onView(allOf(isDisplayed(), withId(R.id.checkbox),
hasSibling(hasDescendant(withText("Choice 1")))))
.perform(click());
onView(allOf(withClassName(is(RadioButton.class.getName())), withParentIndex(3),
withEffectiveVisibility(VISIBLE)))
.perform(click());
onView(allOf(withClassName(is(RadioButton.class.getName())), withParentIndex(0),
withEffectiveVisibility(VISIBLE)))
.perform(click());
// Check that choice 1 is visually selected and choice 2 is de-selected.
onView(allOf(withClassName(is(RadioButton.class.getName())), withParentIndex(0),
withEffectiveVisibility(VISIBLE)))
.check(matches(isChecked()));
onView(allOf(withClassName(is(RadioButton.class.getName())), withParentIndex(3),
withEffectiveVisibility(VISIBLE)))
.check(matches(not(isChecked())));
// Click on Counter 2 +, increase from 0 to 1.
onView(allOf(isDisplayed(), withId(R.id.increase_button),
hasSibling(hasDescendant(withText("Counter 2")))))
......@@ -265,9 +240,8 @@ public class AutofillAssistantFormActionTest {
// Choice 1
assertThat(formResult.get(1).getInputTypeCase(),
is(FormInputProto.Result.InputTypeCase.SELECTION));
assertThat(formResult.get(1).getSelection().getSelectedCount(), is(2));
assertThat(formResult.get(1).getSelection().getSelectedCount(), is(1));
assertThat(formResult.get(1).getSelection().getSelected(0), is(true));
assertThat(formResult.get(1).getSelection().getSelected(1), is(false));
// Counter 3
assertThat(formResult.get(2).getInputTypeCase(),
......
chrome/android/features/autofill_assistant/javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiTestUtil.java
View file @ b3f39033
......@@ -222,26 +222,6 @@ class AutofillAssistantUiTestUtil {
};
}
public static Matcher<View> withParentIndex(int parentIndex) {
return new TypeSafeMatcher<View>() {
@Override
public void describeTo(Description description) {
description.appendText("withParentIndex: " + parentIndex);
}
@Override
public boolean matchesSafely(View view) {
ViewParent parent = view.getParent();
if (!(parent instanceof ViewGroup)) {
return false;
} else {
ViewGroup parentGroup = (ViewGroup) parent;
return parentGroup.getChildAt(parentIndex) == view;
}
}
};
}
public static ViewAction openTextLink(String textLink) {
return new ViewAction() {
@Override
......
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