From 758f4c13ec001b7735c42d65e22b359cc3c9d4e5 Mon Sep 17 00:00:00 2001 From: Colin Lee Date: Thu, 5 Sep 2019 13:01:43 -0500 Subject: [PATCH] Updates per UX and PR feedback --- .../settings/DeleteBrowsingDataController.kt | 7 +++- .../settings/DeleteBrowsingDataFragment.kt | 34 ++++++++----------- ...DefaultDeleteBrowsingDataControllerTest.kt | 2 +- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/settings/DeleteBrowsingDataController.kt b/app/src/main/java/org/mozilla/fenix/settings/DeleteBrowsingDataController.kt index cbaee2879..46d0ef23e 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/DeleteBrowsingDataController.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/DeleteBrowsingDataController.kt @@ -10,6 +10,7 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.withContext import mozilla.components.concept.engine.Engine import mozilla.components.feature.tab.collections.TabCollection +import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.ext.components import kotlin.coroutines.CoroutineContext @@ -34,7 +35,11 @@ class DefaultDeleteBrowsingDataController( override suspend fun deleteBrowsingData() { withContext(coroutineContext) { - context.components.core.engine.clearData(Engine.BrowsingData.all()) + if (FeatureFlags.granularDataDeletion) { + context.components.core.engine.clearData(Engine.BrowsingData.select(Engine.BrowsingData.DOM_STORAGES)) + } else { + context.components.core.engine.clearData(Engine.BrowsingData.all()) + } } context.components.core.historyStorage.deleteEverything() } diff --git a/app/src/main/java/org/mozilla/fenix/settings/DeleteBrowsingDataFragment.kt b/app/src/main/java/org/mozilla/fenix/settings/DeleteBrowsingDataFragment.kt index 978611272..7c5a88cb9 100644 --- a/app/src/main/java/org/mozilla/fenix/settings/DeleteBrowsingDataFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/settings/DeleteBrowsingDataFragment.kt @@ -15,15 +15,12 @@ import androidx.fragment.app.Fragment import androidx.lifecycle.Observer import androidx.lifecycle.lifecycleScope import androidx.navigation.fragment.findNavController -import androidx.paging.PagedList -import androidx.paging.toLiveData import kotlinx.android.synthetic.main.fragment_delete_browsing_data.* import kotlinx.android.synthetic.main.fragment_delete_browsing_data.view.* import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import mozilla.components.browser.session.Session import mozilla.components.browser.session.SessionManager -import mozilla.components.feature.sitepermissions.SitePermissions import mozilla.components.feature.tab.collections.TabCollection import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.R @@ -64,9 +61,17 @@ class DeleteBrowsingDataFragment : Fragment() { }) } - getCheckboxes().forEach { - it.onCheckListener = { _ -> updateDeleteButton() } + if (!FeatureFlags.granularDataDeletion) { + // Disabling the disabled state until we have APIs to decide + // if there is data to delete for all categories + getCheckboxes().forEach { + it.onCheckListener = { _ -> updateCheckboxState() } + } + } else { + // Otherwise, all checkboxes should default to checked state + getCheckboxes().forEach { it.isChecked = true } } + view.delete_data?.setOnClickListener { askToDelete() } @@ -167,7 +172,7 @@ class DeleteBrowsingDataFragment : Fragment() { updateSitePermissions() } - private fun updateDeleteButton() { + private fun updateCheckboxState() { val enabled = getCheckboxes().any { it.isChecked } view?.delete_data?.isEnabled = enabled @@ -181,7 +186,7 @@ class DeleteBrowsingDataFragment : Fragment() { R.string.preferences_delete_browsing_data_tabs_subtitle, openTabs ) - isEnabled = openTabs > 0 + if (!FeatureFlags.granularDataDeletion) isEnabled = openTabs > 0 } } @@ -197,7 +202,7 @@ class DeleteBrowsingDataFragment : Fragment() { R.string.preferences_delete_browsing_data_browsing_data_subtitle, historyCount ) - isEnabled = historyCount > 0 + if (!FeatureFlags.granularDataDeletion) isEnabled = historyCount > 0 } } } @@ -216,7 +221,7 @@ class DeleteBrowsingDataFragment : Fragment() { R.string.preferences_delete_browsing_data_collections_subtitle, collectionsCount ) - isEnabled = collectionsCount > 0 + if (!FeatureFlags.granularDataDeletion) isEnabled = collectionsCount > 0 } } } @@ -231,16 +236,7 @@ class DeleteBrowsingDataFragment : Fragment() { } private fun updateSitePermissions() { - val liveData = - requireComponents.core.permissionStorage.getSitePermissionsPaged().toLiveData(1) - liveData.observe( - this, - object : Observer> { - override fun onChanged(list: PagedList?) { - view?.site_permissions_item?.isEnabled = !list.isNullOrEmpty() - liveData.removeObserver(this) - } - }) + // NO OP until we have GeckoView methods for cookies and cached files, for consistency } private fun getCheckboxes(): List { diff --git a/app/src/test/java/org/mozilla/fenix/settings/DefaultDeleteBrowsingDataControllerTest.kt b/app/src/test/java/org/mozilla/fenix/settings/DefaultDeleteBrowsingDataControllerTest.kt index b43b0ab4f..a970177ed 100644 --- a/app/src/test/java/org/mozilla/fenix/settings/DefaultDeleteBrowsingDataControllerTest.kt +++ b/app/src/test/java/org/mozilla/fenix/settings/DefaultDeleteBrowsingDataControllerTest.kt @@ -74,7 +74,7 @@ class DefaultDeleteBrowsingDataControllerTest { controller.deleteBrowsingData() verify { - context.components.core.engine.clearData(Engine.BrowsingData.all()) + context.components.core.engine.clearData(any()) context.components.core.historyStorage } }