Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 87ea7ae2 authored by Simon Wingrove's avatar Simon Wingrove
Browse files

Fix padding and touch target size on BannerMessage.

Padding between title and subtitle is 8dp should be 4dp.

Touch target on dismiss button is too small for a11y requirements. Use
a touch delegate on the card view to increase touchable size without
changing layout.

Bug: 190780536
Test: Manually confirmed new layout works
Test: Use adb shell input tap x y to touch dismiss button
Change-Id: I749ab2e7cba811d841804d73c13a2b792591dfa1
parent 7dfe7b74
Loading
Loading
Loading
Loading
+5 −2
Original line number Original line Diff line number Diff line
@@ -15,7 +15,7 @@
  limitations under the License.
  limitations under the License.
  -->
  -->


<LinearLayout
<com.android.settingslib.widget.BannerMessageView
    xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:android="http://schemas.android.com/apk/res/android"
    android:layout_width="match_parent"
    android:layout_width="match_parent"
    android:layout_height="wrap_content"
    android:layout_height="wrap_content"
@@ -23,6 +23,7 @@
    style="@style/Banner.Preference.SettingsLib">
    style="@style/Banner.Preference.SettingsLib">


    <RelativeLayout
    <RelativeLayout
        android:id="@+id/top_row"
        android:layout_width="match_parent"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        android:layout_height="wrap_content"
        android:paddingBottom="8dp"
        android:paddingBottom="8dp"
@@ -49,6 +50,7 @@
        android:id="@+id/banner_title"
        android:id="@+id/banner_title"
        android:layout_width="wrap_content"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:layout_height="wrap_content"
        android:paddingTop="0dp"
        android:paddingBottom="4dp"
        android:paddingBottom="4dp"
        android:textAppearance="@style/Banner.Title.SettingsLib"/>
        android:textAppearance="@style/Banner.Title.SettingsLib"/>


@@ -56,6 +58,7 @@
        android:id="@+id/banner_subtitle"
        android:id="@+id/banner_subtitle"
        android:layout_width="wrap_content"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:layout_height="wrap_content"
        android:paddingTop="0dp"
        android:paddingBottom="4dp"
        android:paddingBottom="4dp"
        android:textAppearance="@style/Banner.Subtitle.SettingsLib"
        android:textAppearance="@style/Banner.Subtitle.SettingsLib"
        android:visibility="gone"/>
        android:visibility="gone"/>
@@ -87,4 +90,4 @@
            android:layout_height="wrap_content"
            android:layout_height="wrap_content"
            style="@style/Banner.ButtonText.SettingsLib"/>
            style="@style/Banner.ButtonText.SettingsLib"/>
    </LinearLayout>
    </LinearLayout>
</LinearLayout>
</com.android.settingslib.widget.BannerMessageView>
 No newline at end of file
 No newline at end of file
+111 −0
Original line number Original line Diff line number Diff line
/*
 * Copyright (C) 2021 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

package com.android.settingslib.widget;

import android.content.Context;
import android.graphics.Rect;
import android.util.AttributeSet;
import android.view.TouchDelegate;
import android.view.View;
import android.widget.LinearLayout;

import androidx.annotation.Nullable;

/**
 * The view providing {@link BannerMessagePreference}.
 *
 * <p>Callers should not instantiate this view directly but rather through adding a
 * {@link BannerMessagePreference} to a {@code PreferenceScreen}.
 */
public class BannerMessageView extends LinearLayout {
    private Rect mTouchTargetForDismissButton;

    public BannerMessageView(Context context) {
        super(context);
    }

    public BannerMessageView(Context context,
            @Nullable AttributeSet attrs) {
        super(context, attrs);
    }

    public BannerMessageView(Context context, @Nullable AttributeSet attrs, int defStyleAttr) {
        super(context, attrs, defStyleAttr);
    }

    public BannerMessageView(Context context, AttributeSet attrs, int defStyleAttr,
            int defStyleRes) {
        super(context, attrs, defStyleAttr, defStyleRes);
    }

    @Override
    protected void onLayout(boolean changed, int l, int t, int r, int b) {
        super.onLayout(changed, l, t, r, b);
        setupIncreaseTouchTargetForDismissButton();
    }

    private void setupIncreaseTouchTargetForDismissButton() {
        if (mTouchTargetForDismissButton != null) {
            // Already set up
            return;
        }

        // The dismiss button is in the 'top row' RelativeLayout for positioning, but this element
        // does not have enough space to provide large touch targets.  We therefore set the top
        // target on this view.
        View topRow = findViewById(R.id.top_row);
        View dismissButton = findViewById(R.id.banner_dismiss_btn);
        if (topRow == null || dismissButton == null || dismissButton.getVisibility() != VISIBLE) {
            return;
        }

        int minimum =
                getResources()
                        .getDimensionPixelSize(R.dimen.settingslib_preferred_minimum_touch_target);
        int width = dismissButton.getWidth();
        int height = dismissButton.getHeight();
        int widthIncrease = width < minimum ? minimum - width : 0;
        int heightIncrease = height < minimum ? minimum - height : 0;

        // Compute the hit rect of dismissButton within the local co-orindate reference of this view
        // (rather than it's direct parent topRow).
        Rect hitRectWithinTopRow = new Rect();
        dismissButton.getHitRect(hitRectWithinTopRow);
        Rect hitRectOfTopRowWithinThis = new Rect();
        topRow.getHitRect(hitRectOfTopRowWithinThis);
        mTouchTargetForDismissButton = new Rect();
        mTouchTargetForDismissButton.left =
                hitRectOfTopRowWithinThis.left + hitRectWithinTopRow.left;
        mTouchTargetForDismissButton.right =
                hitRectOfTopRowWithinThis.left + hitRectWithinTopRow.right;
        mTouchTargetForDismissButton.top =
                hitRectOfTopRowWithinThis.top + hitRectWithinTopRow.top;
        mTouchTargetForDismissButton.bottom =
                hitRectOfTopRowWithinThis.top + hitRectWithinTopRow.bottom;

        // Adjust the touch target rect to apply the necessary increase in width and height.
        mTouchTargetForDismissButton.left -=
                widthIncrease % 2 == 1 ? (widthIncrease / 2) + 1 : widthIncrease / 2;
        mTouchTargetForDismissButton.top -=
                heightIncrease % 2 == 1 ? (heightIncrease / 2) + 1 : heightIncrease / 2;
        mTouchTargetForDismissButton.right += widthIncrease / 2;
        mTouchTargetForDismissButton.bottom += heightIncrease / 2;

        setTouchDelegate(new TouchDelegate(mTouchTargetForDismissButton, dismissButton));
    }

}
+1 −0
Original line number Original line Diff line number Diff line
@@ -18,4 +18,5 @@
<resources>
<resources>
    <dimen name="app_preference_padding_start">20dp</dimen>
    <dimen name="app_preference_padding_start">20dp</dimen>
    <dimen name="app_icon_min_width">52dp</dimen>
    <dimen name="app_icon_min_width">52dp</dimen>
    <dimen name="settingslib_preferred_minimum_touch_target">48dp</dimen>
</resources>
</resources>
+30 −3
Original line number Original line Diff line number Diff line
@@ -20,7 +20,10 @@ import static com.google.common.truth.Truth.assertThat;


import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verify;
import static org.robolectric.Robolectric.setupActivity;
import static org.robolectric.Shadows.shadowOf;


import android.app.Activity;
import android.content.Context;
import android.content.Context;
import android.graphics.ColorFilter;
import android.graphics.ColorFilter;
import android.graphics.PorterDuff;
import android.graphics.PorterDuff;
@@ -44,8 +47,8 @@ import org.junit.runner.RunWith;
import org.robolectric.Robolectric;
import org.robolectric.Robolectric;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.RuntimeEnvironment;
import org.robolectric.RuntimeEnvironment;
import org.robolectric.Shadows;
import org.robolectric.shadows.ShadowDrawable;
import org.robolectric.shadows.ShadowDrawable;
import org.robolectric.shadows.ShadowTouchDelegate;
import org.robolectric.util.ReflectionHelpers;
import org.robolectric.util.ReflectionHelpers;


@RunWith(RobolectricTestRunner.class)
@RunWith(RobolectricTestRunner.class)
@@ -58,6 +61,9 @@ public class BannerMessagePreferenceTest {


    private boolean mClickListenerCalled = false;
    private boolean mClickListenerCalled = false;
    private final View.OnClickListener mClickListener = v -> mClickListenerCalled = true;
    private final View.OnClickListener mClickListener = v -> mClickListenerCalled = true;
    private final int mMinimumTargetSize =
            RuntimeEnvironment.application.getResources()
                    .getDimensionPixelSize(R.dimen.settingslib_preferred_minimum_touch_target);


    private static final int TEST_STRING_RES_ID =
    private static final int TEST_STRING_RES_ID =
            R.string.accessibility_banner_message_dismiss;
            R.string.accessibility_banner_message_dismiss;
@@ -80,6 +86,23 @@ public class BannerMessagePreferenceTest {
                .isEqualTo("test");
                .isEqualTo("test");
    }
    }


    @Test
    public void onBindViewHolder_andOnLayoutView_dismissButtonTouchDelegate_isCorrectSize() {
        assumeAndroidS();
        mBannerPreference.setTitle("Title");
        mBannerPreference.setDismissButtonOnClickListener(mClickListener);

        mBannerPreference.onBindViewHolder(mHolder);
        setupActivity(Activity.class).setContentView(mRootView);

        assertThat(mRootView.getTouchDelegate()).isNotNull();
        ShadowTouchDelegate delegate = shadowOf(mRootView.getTouchDelegate());
        assertThat(delegate.getBounds().width()).isAtLeast(mMinimumTargetSize);
        assertThat(delegate.getBounds().height()).isAtLeast(mMinimumTargetSize);
        assertThat(delegate.getDelegateView())
                .isEqualTo(mRootView.findViewById(R.id.banner_dismiss_btn));
    }

    @Test
    @Test
    public void onBindViewHolder_whenSummarySet_shouldSetSummary() {
    public void onBindViewHolder_whenSummarySet_shouldSetSummary() {
        mBannerPreference.setSummary("test");
        mBannerPreference.setSummary("test");
@@ -157,7 +180,7 @@ public class BannerMessagePreferenceTest {
        mBannerPreference.onBindViewHolder(mHolder);
        mBannerPreference.onBindViewHolder(mHolder);


        ImageView mIcon = mRootView.findViewById(R.id.banner_icon);
        ImageView mIcon = mRootView.findViewById(R.id.banner_icon);
        ShadowDrawable shadowDrawable = Shadows.shadowOf(mIcon.getDrawable());
        ShadowDrawable shadowDrawable = shadowOf(mIcon.getDrawable());
        assertThat(shadowDrawable.getCreatedFromResId())
        assertThat(shadowDrawable.getCreatedFromResId())
                .isEqualTo(R.drawable.settingslib_ic_cross);
                .isEqualTo(R.drawable.settingslib_ic_cross);
    }
    }
@@ -168,7 +191,7 @@ public class BannerMessagePreferenceTest {
        mBannerPreference.onBindViewHolder(mHolder);
        mBannerPreference.onBindViewHolder(mHolder);


        ImageView mIcon = mRootView.findViewById(R.id.banner_icon);
        ImageView mIcon = mRootView.findViewById(R.id.banner_icon);
        ShadowDrawable shadowDrawable = Shadows.shadowOf(mIcon.getDrawable());
        ShadowDrawable shadowDrawable = shadowOf(mIcon.getDrawable());
        assertThat(shadowDrawable.getCreatedFromResId()).isEqualTo(R.drawable.ic_warning);
        assertThat(shadowDrawable.getCreatedFromResId()).isEqualTo(R.drawable.ic_warning);
    }
    }


@@ -478,11 +501,15 @@ public class BannerMessagePreferenceTest {


    private void assumeAndroidR() {
    private void assumeAndroidR() {
        ReflectionHelpers.setStaticField(Build.VERSION.class, "SDK_INT", 30);
        ReflectionHelpers.setStaticField(Build.VERSION.class, "SDK_INT", 30);
        ReflectionHelpers.setStaticField(Build.VERSION.class, "CODENAME", "R");
        ReflectionHelpers.setStaticField(BannerMessagePreference.class, "IS_AT_LEAST_S", false);
        // Reset view holder to use correct layout.
        // Reset view holder to use correct layout.
    }
    }


    private void assumeAndroidS() {
    private void assumeAndroidS() {
        ReflectionHelpers.setStaticField(Build.VERSION.class, "SDK_INT", 31);
        ReflectionHelpers.setStaticField(Build.VERSION.class, "SDK_INT", 31);
        ReflectionHelpers.setStaticField(Build.VERSION.class, "CODENAME", "S");
        ReflectionHelpers.setStaticField(BannerMessagePreference.class, "IS_AT_LEAST_S", true);
        // Re-inflate view to update layout.
        // Re-inflate view to update layout.
        setUpViewHolder();
        setUpViewHolder();
    }
    }