Unverified Commit 66210469 authored by Gabriel Luong's avatar Gabriel Luong Committed by GitHub
Browse files

For #9548 - Ability to rename top sites (#16285)



* Ability to rename top sites, closes #9548

* Highlight the "Remove" top site action as destructive

With more menu items in the top site contextual menu, it makes sense to differentiate

* Added test for the top site renaming action

* Fixed lint check (wildcard imports, blank spaces)

* Applied suggestions from code review

* Apply suggestions from code review
Co-authored-by: default avatarGabriel Luong <gabriel.luong@gmail.com>

* Implemented dialog click listener with manual dismiss/cancel
Co-authored-by: default avatarLorenzo Stanco <lorenzo.stanco@gmail.com>
parent 20b3c765
......@@ -119,6 +119,31 @@ class TopSitesTest {
}
}
@Test
fun verifyRenameTopSite() {
val defaultWebPage = TestAssetHelper.getGenericAsset(mockWebServer, 1)
val defaultWebPageTitle = "Test_Page_1"
val defaultWebPageTitleNew = "Test_Page_2"
navigationToolbar {
}.enterURLAndEnterToBrowser(defaultWebPage.url) {
}.openThreeDotMenu {
verifyAddFirefoxHome()
}.addToFirefoxHome {
verifySnackBarText("Added to top sites!")
}.openTabDrawer {
}.openNewTab {
}.dismiss {
verifyExistingTopSitesList()
verifyExistingTopSitesTabs(defaultWebPageTitle)
}.openContextMenuOnTopSitesWithTitle(defaultWebPageTitle) {
verifyTopSiteContextMenuItems()
}.renameTopSite(defaultWebPageTitleNew) {
verifyExistingTopSitesList()
verifyExistingTopSitesTabs(defaultWebPageTitleNew)
}
}
@Test
fun verifyRemoveTopSite() {
val defaultWebPage = TestAssetHelper.getGenericAsset(mockWebServer, 1)
......
......@@ -7,6 +7,7 @@
package org.mozilla.fenix.ui.robots
import android.graphics.Bitmap
import android.widget.EditText
import androidx.recyclerview.widget.RecyclerView
import androidx.test.espresso.Espresso.onView
import androidx.test.espresso.NoMatchingViewException
......@@ -38,7 +39,9 @@ import androidx.test.uiautomator.Until
import androidx.test.uiautomator.Until.findObject
import org.hamcrest.CoreMatchers.allOf
import org.hamcrest.CoreMatchers.containsString
import org.hamcrest.CoreMatchers.instanceOf
import org.hamcrest.CoreMatchers.not
import org.hamcrest.Matchers
import org.junit.Assert
import org.mozilla.fenix.R
import org.mozilla.fenix.components.Search
......@@ -445,6 +448,18 @@ class HomeScreenRobot {
return BrowserRobot.Transition()
}
fun renameTopSite(title: String, interact: HomeScreenRobot.() -> Unit): Transition {
onView(withText("Rename"))
.check((matches(withEffectiveVisibility(Visibility.VISIBLE))))
.perform(click())
onView(Matchers.allOf(withId(R.id.top_site_title), instanceOf(EditText::class.java)))
.perform(ViewActions.replaceText(title))
onView(withId(android.R.id.button1)).perform((click()))
HomeScreenRobot().interact()
return Transition()
}
fun removeTopSite(interact: HomeScreenRobot.() -> Unit): Transition {
onView(withText("Remove"))
.check((matches(withEffectiveVisibility(Visibility.VISIBLE))))
......
......@@ -4,6 +4,9 @@
package org.mozilla.fenix.home.sessioncontrol
import android.view.LayoutInflater
import android.widget.EditText
import androidx.appcompat.app.AlertDialog
import androidx.navigation.NavController
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
......@@ -15,6 +18,7 @@ import mozilla.components.feature.tab.collections.TabCollection
import mozilla.components.feature.tab.collections.ext.restore
import mozilla.components.feature.tabs.TabsUseCases
import mozilla.components.feature.top.sites.TopSite
import mozilla.components.support.ktx.android.view.showKeyboard
import mozilla.components.support.ktx.kotlin.isUrl
import org.mozilla.fenix.BrowserDirection
import org.mozilla.fenix.HomeActivity
......@@ -85,6 +89,11 @@ interface SessionControlController {
*/
fun handlePrivateBrowsingLearnMoreClicked()
/**
* @see [TopSiteInteractor.onRenameTopSiteClicked]
*/
fun handleRenameTopSiteClicked(topSite: TopSite)
/**
* @see [TopSiteInteractor.onRemoveTopSiteClicked]
*/
......@@ -281,6 +290,38 @@ class DefaultSessionControlController(
)
}
override fun handleRenameTopSiteClicked(topSite: TopSite) {
activity.let {
val customLayout =
LayoutInflater.from(it).inflate(R.layout.top_sites_rename_dialog, null)
val topSiteLabelEditText: EditText =
customLayout.findViewById(R.id.top_site_title)
topSiteLabelEditText.setText(topSite.title)
AlertDialog.Builder(it).apply {
setTitle(R.string.rename_top_site)
setView(customLayout)
setPositiveButton(R.string.top_sites_rename_dialog_ok) { dialog, _ ->
val newTitle = topSiteLabelEditText.text.toString()
if (newTitle.isNotBlank()) {
viewLifecycleScope.launch(Dispatchers.IO) {
with(activity.components.useCases.topSitesUseCase) {
renameTopSites(topSite, newTitle)
}
}
}
dialog.dismiss()
}
setNegativeButton(R.string.top_sites_rename_dialog_cancel) { dialog, _ ->
dialog.cancel()
}
}.show().also {
topSiteLabelEditText.setSelection(0, topSiteLabelEditText.text.length)
topSiteLabelEditText.showKeyboard()
}
}
}
override fun handleRemoveTopSiteClicked(topSite: TopSite) {
metrics.track(Event.TopSiteRemoved)
if (topSite.url == SupportUtils.POCKET_TRENDING_URL) {
......
......@@ -156,6 +156,13 @@ interface TopSiteInteractor {
*/
fun onOpenInPrivateTabClicked(topSite: TopSite)
/**
* Opens a dialog to rename the given top site. Called when an user clicks on the "Rename" top site menu item.
*
* @param topSite The top site that will be renamed.
*/
fun onRenameTopSiteClicked(topSite: TopSite)
/**
* Removes the given top site. Called when an user clicks on the "Remove" top site menu item.
*
......@@ -210,6 +217,10 @@ class SessionControlInteractor(
controller.handleOpenInPrivateTabClicked(topSite)
}
override fun onRenameTopSiteClicked(topSite: TopSite) {
controller.handleRenameTopSiteClicked(topSite)
}
override fun onRemoveTopSiteClicked(topSite: TopSite) {
controller.handleRemoveTopSiteClicked(topSite)
}
......
......@@ -43,6 +43,9 @@ class TopSiteItemViewHolder(
is TopSiteItemMenu.Item.OpenInPrivateTab -> interactor.onOpenInPrivateTabClicked(
topSite
)
is TopSiteItemMenu.Item.RenameTopSite -> interactor.onRenameTopSiteClicked(
topSite
)
is TopSiteItemMenu.Item.RemoveTopSite -> interactor.onRemoveTopSiteClicked(
topSite
)
......@@ -100,18 +103,24 @@ class TopSiteItemMenu(
) {
sealed class Item {
object OpenInPrivateTab : Item()
object RenameTopSite : Item()
object RemoveTopSite : Item()
}
val menuBuilder by lazy { BrowserMenuBuilder(menuItems) }
private val menuItems by lazy {
listOf(
listOfNotNull(
SimpleBrowserMenuItem(
context.getString(R.string.bookmark_menu_open_in_private_tab_button)
) {
onItemTapped.invoke(Item.OpenInPrivateTab)
},
if (isPinnedSite) SimpleBrowserMenuItem(
context.getString(R.string.rename_top_site)
) {
onItemTapped.invoke(Item.RenameTopSite)
} else null,
SimpleBrowserMenuItem(
if (isPinnedSite) {
context.getString(R.string.remove_top_site)
......
<?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"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:orientation="vertical">
<TextView
android:id="@+id/name_header"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginStart="26dp"
android:layout_marginTop="16dp"
android:layout_marginEnd="24dp"
android:text="@string/top_sites_rename_dialog_title"
android:textAllCaps="true"
android:textAppearance="@style/Body16TextStyle"
android:textColor="?secondaryText" />
<EditText
android:id="@+id/top_site_title"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginStart="24dp"
android:layout_marginEnd="24dp"
android:autofillHints="false"
android:backgroundTint="?neutral"
android:hint="@string/top_site_name_hint"
android:inputType="textCapSentences"
android:singleLine="true"
android:textAlignment="viewStart" />
</LinearLayout>
......@@ -611,8 +611,10 @@
<string name="collection_open_tabs">Open tabs</string>
<!-- Hint for adding name of a collection -->
<string name="collection_name_hint">Collection name</string>
<!-- Text for the menu button to remove a top site -->
<string name="remove_top_site">Remove</string>
<!-- Text for the menu button to rename a top site -->
<string name="rename_top_site">Rename</string>
<!-- Text for the menu button to remove a top site -->
<string name="remove_top_site">Remove</string>
<!-- Text for the menu button to delete a top site from history -->
<string name="delete_from_history">Delete from history</string>
<!-- Postfix for private WebApp titles, placeholder is replaced with app name -->
......@@ -1575,6 +1577,14 @@
<string name="top_sites_max_limit_confirmation_button">OK, Got It</string>
<!-- Label for the show most visited sites preference -->
<string name="top_sites_toggle_top_frecent_sites">Show most visited sites</string>
<!-- Title text displayed in the rename top site dialog. -->
<string name="top_sites_rename_dialog_title">Name</string>
<!-- Hint for renaming title of a top site -->
<string name="top_site_name_hint">Top site name</string>
<!-- Button caption to confirm the renaming of the top site. -->
<string name="top_sites_rename_dialog_ok">OK</string>
<!-- Dialog button text for canceling the rename top site prompt. -->
<string name="top_sites_rename_dialog_cancel">Cancel</string>
<!-- Content description for close button in collection placeholder. -->
<string name="remove_home_collection_placeholder_content_description">Remove</string>
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment