No issue: Missing clear button in edit bookmarks UI

CHANGELOG.md

@@ -34,6 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - #619 - Sets toolbar behavior based on accessibility and if session is loading
 - #1571 - Added a snackbar for undoing bookmark deletion
 - #1079 - Managing site permissions exceptions
+- #1312 - Added clear textfield buttons for editing bookmarks
 
 ### Changed
 - #1429 - Updated site permissions ui for MVP

app/src/main/java/org/mozilla/fenix/library/bookmarks/addfolder/AddBookmarkFolderFragment.kt

@@ -91,7 +91,7 @@ class AddBookmarkFolderFragment : Fragment(), CoroutineScope {
     override fun onOptionsItemSelected(item: MenuItem): Boolean {
         return when (item.itemId) {
             R.id.confirm_add_folder_button -> {
-                if (bookmark_add_folder_title_edit.text.isEmpty()) {
+                if (bookmark_add_folder_title_edit.text.isNullOrEmpty()) {
                     bookmark_add_folder_title_edit.error = getString(R.string.bookmark_empty_title_error)
                     return true
                 }

app/src/main/java/org/mozilla/fenix/library/bookmarks/edit/EditBookmarkFragment.kt

@@ -26,10 +26,10 @@ import io.reactivex.functions.BiFunction
 import io.reactivex.schedulers.Schedulers
 import kotlinx.android.synthetic.main.fragment_edit_bookmark.*
 import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.Dispatchers.IO
 import kotlinx.coroutines.Dispatchers.Main
 import kotlinx.coroutines.Job
+import kotlinx.coroutines.coroutineScope
 import kotlinx.coroutines.launch
 import mozilla.appservices.places.UrlParseFailed
 import mozilla.components.concept.storage.BookmarkInfo
@@ -39,7 +39,6 @@ import org.mozilla.fenix.R
 import org.mozilla.fenix.ext.getColorFromAttr
 import org.mozilla.fenix.ext.requireComponents
 import org.mozilla.fenix.library.bookmarks.BookmarksSharedViewModel
-import java.lang.IllegalArgumentException
 import java.util.concurrent.TimeUnit
 import kotlin.coroutines.CoroutineContext
 
@@ -52,7 +51,7 @@ class EditBookmarkFragment : Fragment(), CoroutineScope {
     private var bookmarkParent: BookmarkNode? = null
 
     override val coroutineContext: CoroutineContext
-        get() = Dispatchers.Main + job
+        get() = Main + job
 
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
@@ -84,8 +83,8 @@ class EditBookmarkFragment : Fragment(), CoroutineScope {
                         bookmark_url_edit.visibility = View.GONE
                         bookmark_url_label.visibility = View.GONE
                     }
-                    BookmarkNodeType.ITEM -> {}
+                    BookmarkNodeType.ITEM -> {
-                    BookmarkNodeType.SEPARATOR -> {}
+                    }
                     else -> throw IllegalArgumentException()
                 }
                 bookmark_name_edit.setText(bookmarkNode!!.title)
@@ -110,7 +109,9 @@ class EditBookmarkFragment : Fragment(), CoroutineScope {
     }
 
     override fun onPause() {
-        updateBookmarkNode(Pair(bookmark_name_edit.text, bookmark_url_edit.text))
+        launch {
+            updateBookmarkNode(Pair(bookmark_name_edit.text.toString(), bookmark_url_edit.text.toString()))
+        }
         super.onPause()
     }
 
@@ -119,19 +120,16 @@ class EditBookmarkFragment : Fragment(), CoroutineScope {
             bookmark_name_edit.textChanges().skipInitialValue(),
             bookmark_url_edit.textChanges().skipInitialValue(),
             BiFunction { name: CharSequence, url: CharSequence ->
-                Pair(name, url)
+                Pair(name.toString(), url.toString())
             })
-            .filter { it.first.isNotBlank() && it.second.isNotBlank() }
+            .filter { it.first.isNotBlank() }
             .debounce(debouncePeriodInMs, TimeUnit.MILLISECONDS)
             .subscribeOn(Schedulers.io())
             .observeOn(AndroidSchedulers.mainThread())
             .`as`(AutoDispose.autoDisposable(AndroidLifecycleScopeProvider.from(this@EditBookmarkFragment)))
             .subscribe {
-                try {
+                launch(IO) {
-                    bookmark_url_edit.error = null
                     updateBookmarkNode(it)
-                } catch (e: UrlParseFailed) {
-                    bookmark_url_edit.error = getString(R.string.bookmark_invalid_url_error)
                 }
             }
     }
@@ -162,17 +160,23 @@ class EditBookmarkFragment : Fragment(), CoroutineScope {
         }
     }
 
-    private fun updateBookmarkNode(pair: Pair<CharSequence, CharSequence>) {
+    private suspend fun updateBookmarkNode(pair: Pair<String, String>) {
-        launch(IO) {
+        try {
             requireComponents.core.bookmarksStorage.updateNode(
                 guidToEdit,
                 BookmarkInfo(
                     sharedViewModel.selectedFolder?.guid ?: bookmarkNode!!.parentGuid,
                     bookmarkNode!!.position,
-                    pair.first.toString(),
+                    pair.first,
-                    if (bookmarkNode?.type == BookmarkNodeType.ITEM) pair.second.toString() else null
+                    if (bookmarkNode?.type == BookmarkNodeType.ITEM) pair.second else null
                 )
             )
+        } catch (e: UrlParseFailed) {
+            coroutineScope {
+                launch(Main) {
+                    bookmark_url_edit.error = getString(R.string.bookmark_invalid_url_error)
+                }
+            }
         }
     }
 

app/src/main/java/org/mozilla/fenix/utils/ClearableEditText.kt

@@ -0,0 +1,50 @@
+/* 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.utils
+
+import android.annotation.SuppressLint
+import android.content.Context
+import android.graphics.PorterDuff
+import android.graphics.PorterDuffColorFilter
+import android.graphics.drawable.Drawable
+import android.util.AttributeSet
+import android.view.MotionEvent
+import androidx.appcompat.widget.AppCompatEditText
+import androidx.core.content.ContextCompat.getColor
+import org.mozilla.fenix.DefaultThemeManager
+import org.mozilla.fenix.R
+
+class ClearableEditText @JvmOverloads
+constructor(
+    context: Context,
+    attrs: AttributeSet? = null,
+    defStyleAttr: Int = R.attr.editTextStyle
+) :
+    AppCompatEditText(context, attrs, defStyleAttr) {
+
+    @SuppressLint("ClickableViewAccessibility")
+    override fun onTouchEvent(event: MotionEvent?): Boolean {
+        if (length() != 0 && event?.action == MotionEvent.ACTION_UP &&
+            event.rawX >= (this@ClearableEditText.right - this@ClearableEditText.compoundPaddingRight)
+        ) {
+            this@ClearableEditText.setText("")
+            return true
+        }
+        return super.onTouchEvent(event)
+    }
+
+    override fun onTextChanged(text: CharSequence?, start: Int, lengthBefore: Int, lengthAfter: Int) {
+        super.onTextChanged(text, start, lengthBefore, lengthAfter)
+        if (lengthAfter != 0 && error == null) {
+            setCompoundDrawablesWithIntrinsicBounds(0, 0, R.drawable.ic_clear, 0)
+            for (drawable: Drawable in compoundDrawables.filterNotNull()) {
+                val color = DefaultThemeManager.resolveAttribute(R.attr.primaryText, context!!)
+                drawable.colorFilter = PorterDuffColorFilter(getColor(context, color), PorterDuff.Mode.SRC_IN)
+            }
+        } else {
+            setCompoundDrawablesWithIntrinsicBounds(0, 0, 0, 0)
+        }
+    }
+}

app/src/main/res/drawable/ic_clear.xml

@@ -0,0 +1,13 @@
+<?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/. -->
+<vector xmlns:android="http://schemas.android.com/apk/res/android"
+    android:width="16dp"
+    android:height="16dp"
+    android:viewportWidth="16"
+    android:viewportHeight="16">
+  <path
+      android:fillColor="?primaryText"
+      android:pathData="M8,0a8,8 0,1 0,8 8,8 8,0 0,0 -8,-8zM12.243,10.828a1,1 0,0 1,-1.415 1.415L8,9.414l-2.828,2.829a1,1 0,0 1,-1.415 -1.415L6.586,8 3.757,5.172a1,1 0,0 1,1.415 -1.415L8,6.586l2.828,-2.829a1,1 0,0 1,1.415 1.415L9.414,8z"/>
+</vector>

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

@@ -17,9 +17,10 @@
         android:text="@string/bookmark_name_label"
         android:textAllCaps="true"
         android:textColor="?primaryText"
-        android:textSize="12sp" />
+        android:textSize="12sp"
+        android:labelFor="@id/bookmark_add_folder_title_edit" />
 
-    <EditText
+    <org.mozilla.fenix.utils.ClearableEditText
         android:id="@+id/bookmark_add_folder_title_edit"
         android:layout_width="match_parent"
         android:layout_height="wrap_content"

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

@@ -17,9 +17,10 @@
         android:text="@string/bookmark_name_label"
         android:textColor="?primaryText"
         android:textSize="12sp"
-        android:textAllCaps="true"/>
+        android:textAllCaps="true"
+        android:labelFor="@id/bookmark_name_edit" />
 
-    <EditText
+    <org.mozilla.fenix.utils.ClearableEditText
         android:id="@+id/bookmark_name_edit"
         android:layout_width="match_parent"
         android:layout_height="wrap_content"
@@ -27,7 +28,7 @@
         android:textSize="15sp"
         android:textColor="?secondaryText"
         tools:text="Internet for people, not profit -- Mozilla"
-        android:inputType="textAutoComplete"/>
+        android:inputType="textAutoComplete" />
 
     <TextView
         android:id="@+id/bookmark_url_label"
@@ -37,9 +38,10 @@
         android:text="@string/bookmark_url_label"
         android:textColor="?primaryText"
         android:textSize="12sp"
-        android:textAllCaps="true"/>
+        android:textAllCaps="true"
+        android:labelFor="@id/bookmark_url_edit" />
 
-    <EditText
+    <org.mozilla.fenix.utils.ClearableEditText
         android:id="@+id/bookmark_url_edit"
         android:layout_width="match_parent"
         android:layout_height="wrap_content"
@@ -57,7 +59,8 @@
         android:text="@string/bookmark_folder_label"
         android:textColor="?primaryText"
         android:textSize="12sp"
-        android:textAllCaps="true"/>
+        android:textAllCaps="true"
+        android:labelFor="@id/bookmark_folder_selector" />
 
     <TextView
         android:id="@+id/bookmark_folder_selector"