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

Commit a59f9fb0 authored by Jacky Wang's avatar Jacky Wang
Browse files

Fix MainSwitchPreference 

This change simplifies MainSwitchPreference widget significantly and fix
some issues:

- Override setTitle/setSummary/etc to update UI is incorrect.
  Under the hood, notifyChange will be invoked when any change is
  detected and onBindViewHolder is triggered. All the UI binding logic
  must be done within onBindViewHolder.
- The listener is added to MainSwitchBar but never be removed, which
  will cause weird issue (e.g. ag/3645453) when view holder is reused.

Bug: 391092187
Flag: EXEMPT library
Test: atest&manual
Change-Id: I4fc5ad382253c7dade1165487142078e56164941
parent d59304db

packages/SettingsLib/MainSwitchPreference/src/com/android/settingslib/widget/MainSwitchBar.java

+5 −0
Original line number Diff line number Diff line
@@ -240,6 +240,11 @@ public class MainSwitchBar extends LinearLayout implements OnCheckedChangeListen 
        }

    }



    /** Removes all [OnCheckedChangeListener]s. */

    public void removeAllOnSwitchChangeListeners() {

        mSwitchChangeListeners.clear();

    }



    /**

     * Remove a listener for switch changes

     */

packages/SettingsLib/MainSwitchPreference/src/com/android/settingslib/widget/MainSwitchPreference.java

+35 −104
Original line number Diff line number Diff line
@@ -17,12 +17,13 @@ 
package com.android.settingslib.widget;



import android.content.Context;

import android.content.res.TypedArray;

import android.os.Build;

import android.util.AttributeSet;

import android.widget.CompoundButton;

import android.widget.CompoundButton.OnCheckedChangeListener;



import androidx.annotation.NonNull;

import androidx.annotation.Nullable;

import androidx.preference.PreferenceViewHolder;

import androidx.preference.TwoStatePreference;
@@ -34,124 +35,67 @@ import java.util.List; 
/**

 * MainSwitchPreference is a Preference with a customized Switch.

 * This component is used as the main switch of the page

 * to enable or disable the prefereces on the page.

 * to enable or disable the preferences on the page.

 */

public class MainSwitchPreference extends TwoStatePreference

        implements OnCheckedChangeListener, GroupSectionDividerMixin {

public class MainSwitchPreference extends TwoStatePreference implements OnCheckedChangeListener,

        GroupSectionDividerMixin {



    private final List<OnCheckedChangeListener> mSwitchChangeListeners = new ArrayList<>();



    private MainSwitchBar mMainSwitchBar;



    public MainSwitchPreference(Context context) {

        super(context);

        init(context, null);

    public MainSwitchPreference(@NonNull Context context) {

        this(context, null);

    }



    public MainSwitchPreference(Context context, AttributeSet attrs) {

        super(context, attrs);

        init(context, attrs);

    public MainSwitchPreference(@NonNull Context context, @Nullable AttributeSet attrs) {

        this(context, attrs, 0);

    }



    public MainSwitchPreference(Context context, AttributeSet attrs, int defStyleAttr) {

        super(context, attrs, defStyleAttr);

        init(context, attrs);

    public MainSwitchPreference(@NonNull Context context, @Nullable AttributeSet attrs,

            int defStyleAttr) {

        this(context, attrs, defStyleAttr, 0);

    }



    public MainSwitchPreference(Context context, AttributeSet attrs, int defStyleAttr,

            int defStyleRes) {

    public MainSwitchPreference(@NonNull Context context, @Nullable AttributeSet attrs,

            int defStyleAttr, int defStyleRes) {

        super(context, attrs, defStyleAttr, defStyleRes);

        init(context, attrs);

        boolean isExpressive = SettingsThemeHelper.isExpressiveTheme(context);

        int resId = isExpressive ? R.layout.settingslib_expressive_main_switch_layout

                : R.layout.settingslib_main_switch_layout;

        setLayoutResource(resId);

    }



    @Override

    public void onBindViewHolder(PreferenceViewHolder holder) {

    public void onBindViewHolder(@NonNull PreferenceViewHolder holder) {

        super.onBindViewHolder(holder);



        holder.setDividerAllowedAbove(false);

        holder.setDividerAllowedBelow(false);



        mMainSwitchBar = (MainSwitchBar) holder.findViewById(R.id.settingslib_main_switch_bar);

        MainSwitchBar mainSwitchBar = holder.itemView.requireViewById(

                R.id.settingslib_main_switch_bar);

        mainSwitchBar.setTitle(getTitle());

        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.VANILLA_ICE_CREAM) {

            mainSwitchBar.setSummary(getSummary());

        }

        mainSwitchBar.setIconSpaceReserved(isIconSpaceReserved());

        // To support onPreferenceChange callback, it needs to call callChangeListener() when

        // MainSwitchBar is clicked.

        mMainSwitchBar.setOnClickListener((view) -> callChangeListener(isChecked()));

        setIconSpaceReserved(isIconSpaceReserved());

        updateStatus(isChecked());

        registerListenerToSwitchBar();

    }

        mainSwitchBar.setOnClickListener(view -> callChangeListener(isChecked()));



    private void init(Context context, AttributeSet attrs) {

        boolean isExpressive = SettingsThemeHelper.isExpressiveTheme(context);

        int resId = isExpressive

                ? R.layout.settingslib_expressive_main_switch_layout

                : R.layout.settingslib_main_switch_layout;

        setLayoutResource(resId);

        mSwitchChangeListeners.add(this);

        if (attrs != null) {

            final TypedArray a = context.obtainStyledAttributes(attrs,

                    androidx.preference.R.styleable.Preference, 0 /*defStyleAttr*/,

                    0 /*defStyleRes*/);

            final CharSequence title = a.getText(

                    androidx.preference.R.styleable.Preference_android_title);

            setTitle(title);



            CharSequence summary = a.getText(

                    androidx.preference.R.styleable.Preference_android_summary);

            setSummary(summary);



            final boolean bIconSpaceReserved = a.getBoolean(

                    androidx.preference.R.styleable.Preference_android_iconSpaceReserved, true);

            setIconSpaceReserved(bIconSpaceReserved);

            a.recycle();

        }

    }

        // Remove all listeners to 1. avoid triggering listener when update UI 2. prevent potential

        // listener leaking when the view holder is reused by RecyclerView

        mainSwitchBar.removeAllOnSwitchChangeListeners();

        mainSwitchBar.setChecked(isChecked());

        mainSwitchBar.addOnSwitchChangeListener(this);



    @Override

    public void setChecked(boolean checked) {

        super.setChecked(checked);

        if (mMainSwitchBar != null && mMainSwitchBar.isChecked() != checked) {

            mMainSwitchBar.setChecked(checked);

        }

    }



    @Override

    public void setTitle(CharSequence title) {

        super.setTitle(title);

        if (mMainSwitchBar != null) {

            mMainSwitchBar.setTitle(title);

        }

    }



    @Override

    public void setSummary(CharSequence summary) {

        super.setSummary(summary);

        if (mMainSwitchBar != null

                && Build.VERSION.SDK_INT >= Build.VERSION_CODES.VANILLA_ICE_CREAM) {

            mMainSwitchBar.setSummary(summary);

        }

    }



    @Override

    public void setIconSpaceReserved(boolean iconSpaceReserved) {

        super.setIconSpaceReserved(iconSpaceReserved);

        if (mMainSwitchBar != null) {

            mMainSwitchBar.setIconSpaceReserved(iconSpaceReserved);

        }

        mainSwitchBar.show();

    }



    @Override

    public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {

        super.setChecked(isChecked);

    }



    /**

     * Update the switch status of preference

     */

    public void updateStatus(boolean checked) {

        setChecked(checked);

        if (mMainSwitchBar != null) {

            mMainSwitchBar.setTitle(getTitle());

            mMainSwitchBar.show();

        for (OnCheckedChangeListener listener : mSwitchChangeListeners) {

            listener.onCheckedChanged(buttonView, isChecked);

        }

    }
@@ -162,10 +106,6 @@ public class MainSwitchPreference extends TwoStatePreference 
        if (!mSwitchChangeListeners.contains(listener)) {

            mSwitchChangeListeners.add(listener);

        }



        if (mMainSwitchBar != null) {

            mMainSwitchBar.addOnSwitchChangeListener(listener);

        }

    }



    /**
@@ -173,14 +113,5 @@ public class MainSwitchPreference extends TwoStatePreference 
     */

    public void removeOnSwitchChangeListener(OnCheckedChangeListener listener) {

        mSwitchChangeListeners.remove(listener);

        if (mMainSwitchBar != null) {

            mMainSwitchBar.removeOnSwitchChangeListener(listener);

        }

    }



    private void registerListenerToSwitchBar() {

        for (OnCheckedChangeListener listener : mSwitchChangeListeners) {

            mMainSwitchBar.addOnSwitchChangeListener(listener);

        }

    }

}

packages/SettingsLib/tests/robotests/src/com/android/settingslib/satellite/SatelliteDialogUtilsTest.kt

+1 −1
Original line number Diff line number Diff line
@@ -67,7 +67,7 @@ class SatelliteDialogUtilsTest { 


    @Test

    @RequiresFlagsEnabled(Flags.FLAG_OEM_ENABLED_SATELLITE_FLAG)

    fun mayStartSatelliteWarningDialog_satelliteIsOn_showWarningDialog() = runBlocking {

    fun mayStartSatelliteWarningDialog_satelliteIsOn_showWarningDialog(): Unit = runBlocking {

        `when`(satelliteManager.registerForModemStateChanged(any(), any()))

                .thenAnswer { invocation ->

                    val callback = invocation

packages/SettingsLib/tests/robotests/src/com/android/settingslib/widget/MainSwitchPreferenceTest.java

+10 −12
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ import android.view.View; 
import android.widget.TextView;



import androidx.preference.PreferenceViewHolder;

import androidx.test.core.app.ApplicationProvider;



import com.android.settingslib.widget.mainswitch.R;
@@ -30,19 +31,17 @@ import org.junit.Before; 
import org.junit.Test;

import org.junit.runner.RunWith;

import org.robolectric.RobolectricTestRunner;

import org.robolectric.RuntimeEnvironment;



@RunWith(RobolectricTestRunner.class)

public class MainSwitchPreferenceTest {



    private Context mContext;

    private final Context mContext = ApplicationProvider.getApplicationContext();

    private View mRootView;

    private PreferenceViewHolder mHolder;

    private MainSwitchPreference mPreference;



    @Before

    public void setUp() {

        mContext = RuntimeEnvironment.application;

        mRootView = View.inflate(mContext, R.layout.settingslib_main_switch_layout,

                null /* parent */);

        mHolder = PreferenceViewHolder.createInstanceForTests(mRootView);
@@ -50,23 +49,22 @@ public class MainSwitchPreferenceTest { 
    }



    @Test

    public void setTitle_shouldUpdateTitle() {

    public void onBindViewHolder_title() {

        final String defaultOnText = "Test title";



        mPreference.onBindViewHolder(mHolder);

        mPreference.setTitle(defaultOnText);

        mPreference.updateStatus(true /* checked */);

        mPreference.onBindViewHolder(mHolder);



        assertThat(((TextView) mRootView.findViewById(R.id.switch_text)).getText())

                .isEqualTo(defaultOnText);

        assertThat(mRootView.<TextView>requireViewById(

                R.id.switch_text).getText().toString()).isEqualTo(defaultOnText);

    }



    @Test

    public void updateStatus_shouldMatchTheStatus() {

    public void onBindViewHolder_checked() {

        mPreference.setChecked(true);

        mPreference.onBindViewHolder(mHolder);

        mPreference.updateStatus(true);



        assertThat(mPreference.isChecked()).isTrue();

        assertThat(mRootView.<MainSwitchBar>requireViewById(

                R.id.settingslib_main_switch_bar).isChecked()).isTrue();

    }



}