commit 4eee14a8bba5e2a058882ea55c35c0680fa4e13c
parent 577185e60ee2ab7af834102130c2459de80a586d
Author: Moyin <madeyemi@mozilla.com>
Date: Tue, 16 Dec 2025 17:32:26 +0000
Bug 2002551 - Retain options order when selecting Toolbar shortcut r=android-reviewers,RJ
- Updated the Shortcut options to use a `RadioButtonPreference`
- Reduced code duplication by using dynamic layouts
Differential Revision: https://phabricator.services.mozilla.com/D275795
Diffstat:
6 files changed, 67 insertions(+), 195 deletions(-)
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/CustomizationFragment.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/CustomizationFragment.kt
@@ -93,26 +93,35 @@ class CustomizationFragment : PreferenceFragmentCompat() {
}
private fun updateToolbarShortcut() {
- val simpleCategory = requirePreference<PreferenceCategory>(
- R.string.pref_key_customization_category_toolbar_simple_shortcut,
- )
- val expandedCategory = requirePreference<PreferenceCategory>(
- R.string.pref_key_customization_category_toolbar_expanded_shortcut,
+ val category = requirePreference<PreferenceCategory>(
+ R.string.pref_key_customization_category_toolbar_shortcut,
)
val settings = requireContext().settings()
val isExpandedToolbarEnabled = settings.shouldUseExpandedToolbar && isTallWindow() && !isWideWindow()
-
- simpleCategory.isVisible =
- settings.shouldShowToolbarCustomization &&
- settings.shouldUseComposableToolbar &&
- settings.toolbarRedesignEnabled &&
- !isExpandedToolbarEnabled
-
- expandedCategory.isVisible =
- settings.shouldShowToolbarCustomization &&
- settings.shouldUseComposableToolbar &&
- settings.toolbarRedesignEnabled &&
- isExpandedToolbarEnabled
+ val shouldShowShortcutCategory = settings.shouldShowToolbarCustomization &&
+ settings.shouldUseComposableToolbar &&
+ settings.toolbarRedesignEnabled
+
+ category.isVisible = shouldShowShortcutCategory
+ if (shouldShowShortcutCategory) {
+ val shortcutPreference = if (isExpandedToolbarEnabled) {
+ ToolbarExpandedShortcutPreference(requireContext()).apply {
+ key = getString(R.string.pref_key_toolbar_expanded_shortcut)
+ layoutResource = R.layout.preference_toolbar_shortcut
+ }
+ } else {
+ ToolbarSimpleShortcutPreference(requireContext()).apply {
+ key = getString(R.string.pref_key_toolbar_simple_shortcut)
+ layoutResource = R.layout.preference_toolbar_shortcut
+ }
+ }
+ category.apply {
+ removeAll()
+ addPreference(shortcutPreference)
+ val shortcutOptions = shortcutPreference.getShortcutOptions()
+ shortcutOptions.forEach(::addPreference)
+ }
+ }
}
private fun setupRadioGroups() {
diff --git a/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/ToolbarShortcutPreference.kt b/mobile/android/fenix/app/src/main/java/org/mozilla/fenix/settings/ToolbarShortcutPreference.kt
@@ -5,23 +5,17 @@
package org.mozilla.fenix.settings
import android.content.Context
-import android.content.res.ColorStateList
import android.util.AttributeSet
-import android.view.LayoutInflater
import android.view.View
-import android.view.ViewGroup
import android.widget.ImageView
-import android.widget.LinearLayout
-import android.widget.RadioButton
-import android.widget.TextView
import androidx.annotation.AttrRes
import androidx.annotation.ColorInt
-import androidx.core.widget.ImageViewCompat
import androidx.preference.Preference
import androidx.preference.PreferenceViewHolder
import com.google.android.material.color.MaterialColors
import org.mozilla.fenix.GleanMetrics.CustomizationSettings
import org.mozilla.fenix.R
+import org.mozilla.fenix.utils.view.addToRadioGroup
import com.google.android.material.R as materialR
/**
@@ -57,105 +51,53 @@ internal abstract class ToolbarShortcutPreference @JvmOverloads constructor(
super.onBindViewHolder(holder)
val selectedIcon = getSelectedIconImageView(holder)
- val selectedContainer = holder.findViewById(R.id.selected_container) as LinearLayout
- val optionsContainer = holder.findViewById(R.id.options_container) as LinearLayout
- val separator = holder.findViewById(R.id.separator) as View
colorTertiary = holder.itemView.getMaterialColor(materialR.attr.colorTertiary)
colorOnSurface = holder.itemView.getMaterialColor(materialR.attr.colorOnSurface)
colorOnSurfaceVariant = holder.itemView.getMaterialColor(materialR.attr.colorOnSurfaceVariant)
+ selectedIcon.setImageResource(getSelectedOption().icon)
+ }
+
+ private fun getSelectedOption(): ShortcutOption {
val selectedKey = readSelectedKey()
- val selected = options.firstOrNull {
+ return options.firstOrNull {
it.key == ShortcutType.fromValue(selectedKey)
} ?: options.first()
- selectedIcon.setImageResource(selected.icon)
- selectedContainer.removeAllViews()
- selectedContainer.addView(
- makeRow(
- parent = selectedContainer,
- option = selected,
- isChecked = true,
- isEnabled = false,
- onClick = {},
- ),
- )
-
- val remaining = options.filter { it.key != selected.key }.distinctBy { it.key }
- optionsContainer.removeAllViews()
- remaining.forEach { opt ->
- optionsContainer.addView(
- makeRow(
- parent = optionsContainer,
- option = opt,
- isChecked = false,
- isEnabled = true,
- ) { newlySelected ->
- CustomizationSettings.toolbarShortcutSelection.record(
- CustomizationSettings.ToolbarShortcutSelectionExtra(
- toolbarType = getToolbarType(),
- item = newlySelected.key.value,
- ),
- )
- writeSelectedKey(newlySelected.key.value)
- selectedIcon.setImageResource(newlySelected.icon)
- notifyChanged()
- },
- )
- }
+ }
- separator.visibility = if (remaining.isEmpty()) View.GONE else View.VISIBLE
+ @Suppress("SpreadOperator")
+ fun getShortcutOptions(): List<RadioButtonPreference> {
+ val shortcutOptions = options
+ .distinctBy { it.key }
+ .map { newOption ->
+ createShortcutRadioButton(
+ newOption = newOption,
+ selectedOption = getSelectedOption(),
+ )
+ }
+
+ addToRadioGroup(*shortcutOptions.toTypedArray())
+ return shortcutOptions
}
- private fun makeRow(
- parent: ViewGroup,
- option: ShortcutOption,
- isChecked: Boolean,
- isEnabled: Boolean,
- onClick: (ShortcutOption) -> Unit,
- ): View {
- val row = LayoutInflater.from(context)
- .inflate(R.layout.toolbar_shortcut_row, parent, false) as LinearLayout
-
- val radio = row.findViewById<RadioButton>(R.id.row_radio)
- val icon = row.findViewById<ImageView>(R.id.row_icon)
- val label = row.findViewById<TextView>(R.id.row_label)
-
- icon.setImageResource(option.icon)
- label.setText(option.label)
-
- radio?.setStartCheckedIndicator()
- radio.isChecked = isChecked
- radio.isEnabled = true
-
- label.setTextColor(
- if (isChecked) colorTertiary else colorOnSurface,
- )
-
- ImageViewCompat.setImageTintList(
- icon,
- ColorStateList.valueOf(
- if (isChecked) colorTertiary else colorOnSurface,
- ),
- )
-
- radio.buttonTintList = ColorStateList(
- arrayOf(
- intArrayOf(android.R.attr.state_checked),
- intArrayOf(-android.R.attr.state_checked),
- ),
- intArrayOf(colorTertiary, colorOnSurfaceVariant),
- )
-
- row.isEnabled = isEnabled
- row.importantForAccessibility = View.IMPORTANT_FOR_ACCESSIBILITY_YES
-
- if (isEnabled) {
- val clicker = View.OnClickListener { onClick(option) }
- row.setOnClickListener(clicker)
+ private fun createShortcutRadioButton(
+ newOption: ShortcutOption,
+ selectedOption: ShortcutOption,
+ ): RadioButtonPreference = RadioButtonPreference(context).apply {
+ key = newOption.key.value
+ title = context.getString(newOption.label)
+ setCheckedWithoutClickListener(newOption == selectedOption)
+ onClickListener {
+ CustomizationSettings.toolbarShortcutSelection.record(
+ CustomizationSettings.ToolbarShortcutSelectionExtra(
+ toolbarType = getToolbarType(),
+ item = newOption.key.value,
+ ),
+ )
+ writeSelectedKey(newOption.key.value)
+ notifyChanged()
}
-
- return row
}
private fun View.getMaterialColor(
diff --git a/mobile/android/fenix/app/src/main/res/layout/preference_toolbar_shortcut.xml b/mobile/android/fenix/app/src/main/res/layout/preference_toolbar_shortcut.xml
@@ -29,25 +29,4 @@
android:id="@+id/toolbar_expanded_shortcut_preview"
layout="@layout/toolbar_expanded_shortcut_preview"
android:visibility="gone" />
-
- <LinearLayout
- android:id="@+id/selected_container"
- android:layout_width="match_parent"
- android:layout_height="wrap_content"
- android:orientation="vertical"
- android:paddingTop="8dp" />
-
- <View
- android:id="@+id/separator"
- android:layout_width="match_parent"
- android:layout_height="1dp"
- android:alpha="0.12"
- android:background="?attr/colorOnSurface"
- android:visibility="gone" />
-
- <LinearLayout
- android:id="@+id/options_container"
- android:layout_width="match_parent"
- android:layout_height="wrap_content"
- android:orientation="vertical" />
</LinearLayout>
diff --git a/mobile/android/fenix/app/src/main/res/layout/toolbar_shortcut_row.xml b/mobile/android/fenix/app/src/main/res/layout/toolbar_shortcut_row.xml
@@ -1,39 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>
-<!-- This Source Code Form is subject to the terms of the Mozilla Public
- - 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/. -->
-<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
- xmlns:app="http://schemas.android.com/apk/res-auto"
- android:layout_width="match_parent"
- android:layout_height="48dp"
- android:gravity="center_vertical"
- android:orientation="horizontal"
- android:paddingStart="@dimen/top_bar_no_icon_alignment_margin_start"
- android:paddingEnd="16dp"
- android:foreground="?android:attr/selectableItemBackground">
-
- <RadioButton
- android:id="@+id/row_radio"
- android:layout_width="wrap_content"
- android:layout_height="wrap_content"
- android:minWidth="0dp"
- android:padding="0dp"
- android:focusable="false"
- android:clickable="false"
- android:button="@null"/>
-
- <ImageView
- android:id="@+id/row_icon"
- android:layout_width="24dp"
- android:layout_height="24dp"
- android:layout_marginStart="@dimen/top_bar_no_icon_alignment_margin_start"
- android:layout_marginEnd="8dp"
- android:contentDescription="@null" />
-
- <TextView
- android:id="@+id/row_label"
- android:layout_width="0dp"
- android:layout_height="wrap_content"
- android:layout_weight="1"
- android:textAppearance="?android:attr/textAppearanceListItem" />
-</LinearLayout>
diff --git a/mobile/android/fenix/app/src/main/res/values/preference_keys.xml b/mobile/android/fenix/app/src/main/res/values/preference_keys.xml
@@ -207,8 +207,7 @@
<string name="pref_key_customization_category_toolbar" translatable="false">pref_key_customization_category_toolbar</string>
<string name="pref_key_customization_category_toolbar_layout" translatable="false">pref_key_customization_category_toolbar_layout</string>
<string name="pref_key_customization_category_app_icon" translatable="false">pref_key_customization_category_app_icon</string>
- <string name="pref_key_customization_category_toolbar_simple_shortcut" translatable="false">pref_key_customization_category_toolbar_simple_shortcut</string>
- <string name="pref_key_customization_category_toolbar_expanded_shortcut" translatable="false">pref_key_customization_category_toolbar_expanded_shortcut</string>
+ <string name="pref_key_customization_category_toolbar_shortcut" translatable="false">pref_key_customization_category_toolbar_shortcut</string>
<string name="pref_key_toolbar_simple_shortcut" translatable="false">pref_key_toolbar_simple_shortcut</string>
<string name="pref_key_toolbar_expanded_shortcut" translatable="false">pref_key_toolbar_expanded_shortcut</string>
diff --git a/mobile/android/fenix/app/src/main/res/xml/customization_preferences.xml b/mobile/android/fenix/app/src/main/res/xml/customization_preferences.xml
@@ -82,31 +82,13 @@
app:trueOptionTitle="@string/preference_expanded_toolbar" />
</androidx.preference.PreferenceCategory>
- <!-- Simple toolbar shortcut picker -->
+ <!-- Toolbar shortcut picker -->
<androidx.preference.PreferenceCategory
+ android:key="@string/pref_key_customization_category_toolbar_shortcut"
android:layout="@layout/preference_cat_style"
android:title="@string/preferences_toolbar_shortcut"
- android:key="@string/pref_key_customization_category_toolbar_simple_shortcut"
- app:iconSpaceReserved="false">
-
- <!-- Custom preference shows the selected option on top -->
- <org.mozilla.fenix.settings.ToolbarSimpleShortcutPreference
- android:key="@string/pref_key_toolbar_simple_shortcut"
- android:layout="@layout/preference_toolbar_shortcut"/>
- </androidx.preference.PreferenceCategory>
-
- <!-- Expanded toolbar shortcut picker -->
- <androidx.preference.PreferenceCategory
- android:layout="@layout/preference_cat_style"
- android:title="@string/preferences_toolbar_shortcut"
- android:key="@string/pref_key_customization_category_toolbar_expanded_shortcut"
- app:iconSpaceReserved="false">
-
- <!-- Custom preference shows the selected option on top -->
- <org.mozilla.fenix.settings.ToolbarExpandedShortcutPreference
- android:key="@string/pref_key_toolbar_expanded_shortcut"
- android:layout="@layout/preference_toolbar_shortcut"/>
- </androidx.preference.PreferenceCategory>
+ android:visible="false"
+ app:iconSpaceReserved="false" />
<androidx.preference.PreferenceCategory
android:layout="@layout/preference_cat_style"
