For #3278 - Fixes search engine selector layout

app/src/main/java/org/mozilla/fenix/settings/RadioSearchEngineListPreference.kt

@@ -7,15 +7,13 @@ package org.mozilla.fenix.settings
 import android.content.Context
 import android.util.AttributeSet
 import android.widget.CompoundButton
-import android.widget.RadioGroup
 import androidx.preference.PreferenceViewHolder
+import mozilla.components.browser.search.SearchEngine
 import org.mozilla.fenix.R
 import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.utils.Settings
 
-class RadioSearchEngineListPreference : SearchEngineListPreference,
+class RadioSearchEngineListPreference : SearchEngineListPreference {
-    RadioGroup.OnCheckedChangeListener {
-
     override val itemResId: Int
         get() = R.layout.search_engine_radio_button
 
@@ -29,24 +27,14 @@ class RadioSearchEngineListPreference : SearchEngineListPreference,
 
     override fun onBindViewHolder(holder: PreferenceViewHolder?) {
         super.onBindViewHolder(holder)
-        searchEngineGroup!!.setOnCheckedChangeListener(this)
     }
 
     override fun updateDefaultItem(defaultButton: CompoundButton) {
         defaultButton.isChecked = true
     }
 
-    override fun onCheckedChanged(group: RadioGroup, checkedId: Int) {
+    override fun onSearchEngineSelected(searchEngine: SearchEngine) {
-        /* onCheckedChanged is called intermittently before the search engine table is full, so we
+        context.components.search.searchEngineManager.defaultSearchEngine = searchEngine
-           must check these conditions to prevent crashes and inconsistent states. */
+        Settings.getInstance(context).setDefaultSearchEngineByName(searchEngine.name)
-        if (group.childCount != searchEngines.count() || group.getChildAt(checkedId) == null ||
-            !group.getChildAt(checkedId).isPressed
-        ) {
-            return
-        }
-
-        val newDefaultEngine = searchEngines[checkedId]
-        context.components.search.searchEngineManager.defaultSearchEngine = newDefaultEngine
-        Settings.getInstance(group.context).setDefaultSearchEngineByName(newDefaultEngine.name)
     }
 }

app/src/main/java/org/mozilla/fenix/settings/SearchEngineListPreference.kt

@@ -9,12 +9,15 @@ import android.content.res.Resources
 import android.graphics.drawable.BitmapDrawable
 import android.util.AttributeSet
 import android.view.LayoutInflater
+import android.view.View
 import android.view.ViewGroup
 import android.widget.CompoundButton
 import android.widget.RadioGroup
+import androidx.constraintlayout.widget.ConstraintLayout
 import androidx.core.content.ContextCompat
 import androidx.preference.Preference
 import androidx.preference.PreferenceViewHolder
+import kotlinx.android.synthetic.main.search_engine_radio_button.view.*
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.Job
@@ -25,7 +28,7 @@ import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.utils.Settings
 import kotlin.coroutines.CoroutineContext
 
-abstract class SearchEngineListPreference : Preference, CoroutineScope {
+abstract class SearchEngineListPreference : Preference, CompoundButton.OnCheckedChangeListener, CoroutineScope {
     private val job = Job()
     override val coroutineContext: CoroutineContext
         get() = job + Dispatchers.Main
@@ -62,6 +65,7 @@ abstract class SearchEngineListPreference : Preference, CoroutineScope {
         super.onDetached()
     }
 
+    protected abstract fun onSearchEngineSelected(searchEngine: SearchEngine)
     protected abstract fun updateDefaultItem(defaultButton: CompoundButton)
 
     private fun refreshSearchEngineViews(context: Context) {
@@ -93,7 +97,7 @@ abstract class SearchEngineListPreference : Preference, CoroutineScope {
             engineItem.id = i
             engineItem.tag = engineId
             if (engineId == defaultSearchEngine) {
-                updateDefaultItem(engineItem)
+                updateDefaultItem(engineItem.radio_button)
             }
             searchEngineGroup!!.addView(engineItem, layoutParams)
         }
@@ -103,19 +107,38 @@ abstract class SearchEngineListPreference : Preference, CoroutineScope {
         engine: SearchEngine,
         layoutInflater: LayoutInflater,
         res: Resources
-    ): CompoundButton {
+    ): View {
-        val buttonItem = layoutInflater.inflate(itemResId, null) as CompoundButton
+        val wrapper = layoutInflater.inflate(itemResId, null) as ConstraintLayout
-        buttonItem.text = engine.name
+        wrapper.setOnClickListener { wrapper.radio_button.isChecked = true }
+        wrapper.radio_button.setOnCheckedChangeListener(this)
+        val buttonItem = wrapper.radio_button
+        wrapper.engine_text.text = engine.name
         val iconSize = res.getDimension(R.dimen.preference_icon_drawable_size).toInt()
         val engineIcon = BitmapDrawable(res, engine.icon)
         engineIcon.setBounds(0, 0, iconSize, iconSize)
+        wrapper.engine_icon.setImageDrawable(engineIcon)
         val attr =
             ThemeManager.resolveAttribute(android.R.attr.listChoiceIndicatorSingle, context)
         val buttonDrawable = ContextCompat.getDrawable(context, attr)
         buttonDrawable.apply {
             this?.setBounds(0, 0, this.intrinsicWidth, this.intrinsicHeight)
         }
-        buttonItem.setCompoundDrawables(engineIcon, null, buttonDrawable, null)
+        buttonItem.setCompoundDrawablesRelative(buttonDrawable, null, null, null)
-        return buttonItem
+        return wrapper
+    }
+
+    override fun onCheckedChanged(buttonView: CompoundButton, isChecked: Boolean) {
+        searchEngines.forEach { engine ->
+            val wrapper: ConstraintLayout = searchEngineGroup?.findViewWithTag(engine.identifier) ?: return
+
+            when (wrapper.radio_button == buttonView) {
+                true -> onSearchEngineSelected(engine)
+                false -> {
+                    wrapper.radio_button.setOnCheckedChangeListener(null)
+                    wrapper.radio_button.isChecked = false
+                    wrapper.radio_button.setOnCheckedChangeListener(this)
+                }
+            }
+        }
     }
 }

app/src/main/res/layout/search_engine_radio_button.xml

@@ -2,19 +2,40 @@
    - License, v. 2.0. If a copy of the MPL was not distributed with this
    - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
 
-<RadioButton xmlns:android="http://schemas.android.com/apk/res/android"
+
-    android:id="@+id/radio_button"
+<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android"
-    android:layout_width="match_parent"
     android:layout_height="@dimen/search_engine_radio_button_height"
-    android:layout_gravity="start"
+    android:layout_width="match_parent"
+    xmlns:app="http://schemas.android.com/apk/res-auto"
     android:background="?android:selectableItemBackground"
-    android:button="@null"
+    android:clickable="true"
-    android:drawableStart="@drawable/ic_close"
+    android:focusable="true">
-    android:drawableEnd="?android:attr/listChoiceIndicatorSingle"
+    <RadioButton
-    android:drawablePadding="16dp"
+        android:id="@+id/radio_button"
-    android:paddingStart="@dimen/radio_button_padding_horizontal"
+        android:layout_width="@dimen/search_engine_radio_button_height"
-    android:paddingTop="@dimen/radio_button_padding_vertical"
+        android:layout_height="@dimen/search_engine_radio_button_height"
-    android:paddingEnd="@dimen/radio_button_padding_horizontal"
+        android:button="@null"
-    android:paddingBottom="@dimen/radio_button_padding_vertical"
+        android:textAlignment="textStart"
-    android:textAlignment="textStart"
+        android:textAppearance="?android:attr/textAppearanceListItem"
-    android:textAppearance="?android:attr/textAppearanceListItem" />
+        android:layout_marginStart="@dimen/radio_button_padding_horizontal"
+        app:layout_constraintStart_toStartOf="parent"
+        app:layout_constraintTop_toTopOf="parent"
+        app:layout_constraintBottom_toBottomOf="parent"/>
+    <ImageView
+        android:id="@+id/engine_icon"
+        android:importantForAccessibility="no"
+        android:layout_width="24dp"
+        android:layout_height="24dp"
+        app:layout_constraintStart_toEndOf="@id/radio_button"
+        app:layout_constraintTop_toTopOf="@id/radio_button"
+        app:layout_constraintBottom_toBottomOf="@id/radio_button" />
+    <TextView
+        android:id="@+id/engine_text"
+        android:layout_width="0dp"
+        android:layout_height="wrap_content"
+        android:layout_marginStart="@dimen/radio_button_padding_horizontal"
+        android:layout_marginEnd="@dimen/radio_button_padding_horizontal"
+        app:layout_constraintStart_toEndOf="@id/engine_icon"
+        app:layout_constraintTop_toTopOf="@id/engine_icon"
+        app:layout_constraintEnd_toEndOf="parent" />
+</androidx.constraintlayout.widget.ConstraintLayout>