From 432d5fbff49f18240844f3e49aeebce8b279afd7 Mon Sep 17 00:00:00 2001 From: Mihai Branescu Date: Sat, 26 Oct 2019 07:41:48 +0300 Subject: [PATCH] For #5848 Wrong toolbar colour - edit bookmark fragment (#6047) - Moved toolbar coloring to extension method - Refactored classes using it - Removed selection mode colouring for EditBookmarkFragment toolbar, making it only black and white (normal mode) --- .../java/org/mozilla/fenix/ext/Toolbar.kt | 44 +++++++++++++ .../mozilla/fenix/library/LibraryFragment.kt | 34 +++++----- .../mozilla/fenix/library/LibraryPageView.kt | 65 +++++-------------- .../bookmarks/edit/EditBookmarkFragment.kt | 24 +++++-- 4 files changed, 97 insertions(+), 70 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/ext/Toolbar.kt diff --git a/app/src/main/java/org/mozilla/fenix/ext/Toolbar.kt b/app/src/main/java/org/mozilla/fenix/ext/Toolbar.kt new file mode 100644 index 000000000..f10a071e2 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/ext/Toolbar.kt @@ -0,0 +1,44 @@ +/* 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/. */ + +package org.mozilla.fenix.ext + +import android.graphics.ColorFilter +import android.graphics.PorterDuff +import android.graphics.PorterDuffColorFilter +import android.widget.ActionMenuView +import android.widget.ImageButton +import androidx.annotation.ColorInt +import androidx.appcompat.view.menu.ActionMenuItemView +import androidx.appcompat.widget.Toolbar +import androidx.core.view.forEach + +/** + * Adjust the colors of the [Toolbar] on the top of the screen. + */ +fun Toolbar.setToolbarColors(@ColorInt foreground: Int, @ColorInt background: Int) { + apply { + setBackgroundColor(background) + setTitleTextColor(foreground) + + val colorFilter = PorterDuffColorFilter(foreground, PorterDuff.Mode.SRC_IN) + overflowIcon?.colorFilter = colorFilter + forEach { child -> + when (child) { + is ImageButton -> child.drawable.colorFilter = colorFilter + is ActionMenuView -> themeActionMenuView(child, colorFilter) + } + } + } +} + +private fun themeActionMenuView(item: ActionMenuView, colorFilter: ColorFilter) { + item.forEach { innerChild -> + if (innerChild is ActionMenuItemView) { + innerChild.compoundDrawables.forEach { drawable -> + item.post { drawable?.colorFilter = colorFilter } + } + } + } +} diff --git a/app/src/main/java/org/mozilla/fenix/library/LibraryFragment.kt b/app/src/main/java/org/mozilla/fenix/library/LibraryFragment.kt index 78d2ca560..95e4dc670 100644 --- a/app/src/main/java/org/mozilla/fenix/library/LibraryFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/LibraryFragment.kt @@ -4,8 +4,6 @@ package org.mozilla.fenix.library -import android.graphics.PorterDuff.Mode.SRC_IN -import android.graphics.PorterDuffColorFilter import android.os.Bundle import android.view.Menu import android.view.MenuInflater @@ -22,6 +20,7 @@ import org.mozilla.fenix.R import org.mozilla.fenix.components.metrics.Event import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.requireComponents +import org.mozilla.fenix.ext.setToolbarColors /** * Displays buttons to navigate to library sections, such as bookmarks and history. @@ -35,10 +34,7 @@ class LibraryFragment : Fragment(R.layout.fragment_library) { override fun onResume() { super.onResume() - - setToolbarColor() - (activity as AppCompatActivity).title = getString(R.string.library_title) - (activity as AppCompatActivity).supportActionBar?.show() + initToolbar() } override fun onViewCreated(view: View, savedInstanceState: Bundle?) { @@ -47,7 +43,10 @@ class LibraryFragment : Fragment(R.layout.fragment_library) { libraryHistory.setOnClickListener { requireComponents.analytics.metrics .track(Event.LibrarySelectedItem(view.context.getString(R.string.library_history))) - nav(R.id.libraryFragment, LibraryFragmentDirections.actionLibraryFragmentToHistoryFragment()) + nav( + R.id.libraryFragment, + LibraryFragmentDirections.actionLibraryFragmentToHistoryFragment() + ) } libraryBookmarks.setOnClickListener { @@ -81,15 +80,16 @@ class LibraryFragment : Fragment(R.layout.fragment_library) { requireComponents.analytics.metrics.track(Event.LibraryClosed) } - private fun setToolbarColor() { - val toolbar = (activity as AppCompatActivity).findViewById(R.id.navigationToolbar) - - val backgroundColor = context!!.getColorFromAttr(R.attr.foundation) - val foregroundColor = context!!.getColorFromAttr(R.attr.primaryText) - - toolbar.setBackgroundColor(backgroundColor) - toolbar.setTitleTextColor(foregroundColor) - toolbar.navigationIcon?.colorFilter = - PorterDuffColorFilter(foregroundColor, SRC_IN) + private fun initToolbar() { + val activity = activity as? AppCompatActivity + val toolbar = activity?.findViewById(R.id.navigationToolbar) + context?.let { context -> + toolbar?.setToolbarColors( + foreground = context.getColorFromAttr(R.attr.primaryText), + background = context.getColorFromAttr(R.attr.foundation) + ) + } + activity?.title = getString(R.string.library_title) + activity?.supportActionBar?.show() } } diff --git a/app/src/main/java/org/mozilla/fenix/library/LibraryPageView.kt b/app/src/main/java/org/mozilla/fenix/library/LibraryPageView.kt index 879aa7dab..39ffa7b65 100644 --- a/app/src/main/java/org/mozilla/fenix/library/LibraryPageView.kt +++ b/app/src/main/java/org/mozilla/fenix/library/LibraryPageView.kt @@ -5,18 +5,10 @@ package org.mozilla.fenix.library import android.content.Context -import android.graphics.ColorFilter -import android.graphics.PorterDuff -import android.graphics.PorterDuffColorFilter import android.view.ViewGroup -import android.widget.ActionMenuView -import android.widget.ImageButton -import androidx.annotation.ColorInt -import androidx.appcompat.view.menu.ActionMenuItemView import androidx.appcompat.widget.Toolbar import androidx.core.content.ContextCompat import androidx.core.view.children -import androidx.core.view.forEach import androidx.recyclerview.widget.RecyclerView import kotlinx.android.extensions.LayoutContainer import kotlinx.android.synthetic.main.library_site_item.view.* @@ -24,6 +16,7 @@ import org.mozilla.fenix.R import org.mozilla.fenix.ext.asActivity import org.mozilla.fenix.ext.getColorFromAttr import org.mozilla.fenix.ext.hideAndDisable +import org.mozilla.fenix.ext.setToolbarColors import org.mozilla.fenix.ext.showAndEnable open class LibraryPageView( @@ -36,14 +29,14 @@ open class LibraryPageView( title: String?, libraryItemsList: RecyclerView ) { - activity?.title = title - setToolbarColors( - context.getColorFromAttr(R.attr.primaryText), - context.getColorFromAttr(R.attr.foundation) + updateToolbar( + title = title, + foregroundColor = context.getColorFromAttr(R.attr.primaryText), + backgroundColor = context.getColorFromAttr(R.attr.foundation) ) libraryItemsList.setItemViewCacheSize(0) - libraryItemsList.children.forEach { - item -> item.overflow_menu.showAndEnable() + libraryItemsList.children.forEach { item -> + item.overflow_menu.showAndEnable() } } @@ -51,46 +44,20 @@ open class LibraryPageView( title: String?, libraryItemsList: RecyclerView ) { - activity?.title = title - setToolbarColors( - ContextCompat.getColor(context, R.color.white_color), - context.getColorFromAttr(R.attr.accentHighContrast) + updateToolbar( + title = title, + foregroundColor = ContextCompat.getColor(context, R.color.white_color), + backgroundColor = context.getColorFromAttr(R.attr.accentHighContrast) ) libraryItemsList.setItemViewCacheSize(0) - libraryItemsList.children.forEach { - item -> item.overflow_menu.hideAndDisable() + libraryItemsList.children.forEach { item -> + item.overflow_menu.hideAndDisable() } } - /** - * Adjust the colors of the [Toolbar] on the top of the screen. - */ - private fun setToolbarColors(@ColorInt foreground: Int, @ColorInt background: Int) { + private fun updateToolbar(title: String?, foregroundColor: Int, backgroundColor: Int) { + activity?.title = title val toolbar = activity?.findViewById(R.id.navigationToolbar) - - toolbar?.apply { - setBackgroundColor(background) - setTitleTextColor(foreground) - - val colorFilter = PorterDuffColorFilter(foreground, PorterDuff.Mode.SRC_IN) - - overflowIcon?.colorFilter = colorFilter - forEach { child -> - when (child) { - is ImageButton -> child.drawable.colorFilter = colorFilter - is ActionMenuView -> themeActionMenuView(child, colorFilter) - } - } - } - } - - private fun themeActionMenuView(item: ActionMenuView, colorFilter: ColorFilter) { - item.forEach { innerChild -> - if (innerChild is ActionMenuItemView) { - innerChild.compoundDrawables.forEach { drawable -> - item.post { drawable?.colorFilter = colorFilter } - } - } - } + toolbar?.setToolbarColors(foregroundColor, backgroundColor) } } diff --git a/app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt b/app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt index 16380564e..b2e2bbca4 100644 --- a/app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt @@ -12,6 +12,7 @@ import android.view.MenuItem import android.view.View import androidx.appcompat.app.AlertDialog import androidx.appcompat.app.AppCompatActivity +import androidx.appcompat.widget.Toolbar import androidx.fragment.app.Fragment import androidx.fragment.app.activityViewModels import androidx.lifecycle.ViewModelProvider @@ -33,6 +34,7 @@ import mozilla.appservices.places.UrlParseFailed import mozilla.components.concept.storage.BookmarkInfo import mozilla.components.concept.storage.BookmarkNode import mozilla.components.concept.storage.BookmarkNodeType +import mozilla.components.support.ktx.android.content.getColorFromAttr import mozilla.components.support.ktx.android.view.hideKeyboard import org.mozilla.fenix.R import org.mozilla.fenix.components.FenixSnackbar @@ -41,6 +43,7 @@ import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.getRootView import org.mozilla.fenix.ext.nav import org.mozilla.fenix.ext.requireComponents +import org.mozilla.fenix.ext.setToolbarColors import org.mozilla.fenix.ext.urlToTrimmedHost import org.mozilla.fenix.library.bookmarks.BookmarksSharedViewModel import org.mozilla.fenix.library.bookmarks.DesktopFolders @@ -66,8 +69,7 @@ class EditBookmarkFragment : Fragment(R.layout.fragment_edit_bookmark) { override fun onResume() { super.onResume() - val activity = activity as? AppCompatActivity - activity?.supportActionBar?.show() + initToolbar() guidToEdit = EditBookmarkFragmentArgs.fromBundle(arguments!!).guidToEdit lifecycleScope.launch(Main) { @@ -119,6 +121,18 @@ class EditBookmarkFragment : Fragment(R.layout.fragment_edit_bookmark) { updateBookmarkFromObservableInput() } + private fun initToolbar() { + val activity = activity as? AppCompatActivity + val toolbar = activity?.findViewById(R.id.navigationToolbar) + context?.let { + toolbar?.setToolbarColors( + foreground = it.getColorFromAttr(R.attr.primaryText), + background = it.getColorFromAttr(R.attr.foundation) + ) + } + activity?.supportActionBar?.show() + } + override fun onPause() { super.onPause() bookmarkNameEdit.hideKeyboard() @@ -169,12 +183,14 @@ class EditBookmarkFragment : Fragment(R.layout.fragment_edit_bookmark) { requireComponents.analytics.metrics.track(Event.RemoveBookmark) launch(Main) { - Navigation.findNavController(requireActivity(), R.id.container).popBackStack() + Navigation.findNavController(requireActivity(), R.id.container) + .popBackStack() activity.getRootView()?.let { rootView -> bookmarkNode?.let { FenixSnackbar.make(rootView, FenixSnackbar.LENGTH_SHORT) .setText( - getString(R.string.bookmark_deletion_snackbar_message, + getString( + R.string.bookmark_deletion_snackbar_message, it.url?.urlToTrimmedHost(activity) ?: it.title ) )