mirror of
https://github.com/fork-maintainers/iceraven-browser
synced 2024-11-03 23:15:31 +00:00
[fenix] Exclude separators from the bookmarks UI
This commit is contained in:
Grisha Kruglov
Grisha Kruglov
parent
a1dc2c4f10
commit
ba388e3d88
@ -21,23 +21,25 @@ import org.mozilla.fenix.library.bookmarks.viewholders.BookmarkSeparatorViewHold
|
||||
class BookmarkAdapter(private val emptyView: View, private val interactor: BookmarkViewInteractor) :
|
||||
RecyclerView.Adapter<RecyclerView.ViewHolder>() {
|
||||
|
||||
private var tree: List<BookmarkNode> = listOf()
|
||||
@VisibleForTesting var tree: List<BookmarkNode> = listOf()
|
||||
private var mode: BookmarkFragmentState.Mode = BookmarkFragmentState.Mode.Normal()
|
||||
private var isFirstRun = true
|
||||
|
||||
fun updateData(tree: BookmarkNode?, mode: BookmarkFragmentState.Mode) {
|
||||
// Display folders above all other bookmarks.
|
||||
val allNodes = tree?.children.orEmpty()
|
||||
val folders: MutableList<BookmarkNode> = mutableListOf()
|
||||
val notFolders: MutableList<BookmarkNode> = mutableListOf()
|
||||
val separators: MutableList<BookmarkNode> = mutableListOf()
|
||||
allNodes.forEach {
|
||||
if (it.type == BookmarkNodeType.FOLDER) {
|
||||
folders.add(it)
|
||||
} else {
|
||||
notFolders.add(it)
|
||||
when (it.type) {
|
||||
BookmarkNodeType.SEPARATOR -> separators.add(it)
|
||||
BookmarkNodeType.FOLDER -> folders.add(it)
|
||||
else -> notFolders.add(it)
|
||||
}
|
||||
}
|
||||
val newTree = folders + notFolders
|
||||
// Display folders above all other bookmarks. Exclude separators.
|
||||
// For separator removal, see discussion in https://github.com/mozilla-mobile/fenix/issues/15214
|
||||
val newTree = folders + notFolders - separators
|
||||
|
||||
val diffUtil = DiffUtil.calculateDiff(
|
||||
BookmarkDiffUtil(
|
||||
|
||||
@ -8,29 +8,20 @@ import io.mockk.mockk
|
||||
import io.mockk.spyk
|
||||
import io.mockk.verifyOrder
|
||||
import mozilla.components.concept.storage.BookmarkNode
|
||||
import mozilla.components.concept.storage.BookmarkNodeType
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Before
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
|
||||
import org.mozilla.fenix.library.bookmarks.viewholders.BookmarkNodeViewHolder
|
||||
|
||||
@RunWith(FenixRobolectricTestRunner::class)
|
||||
internal class BookmarkAdapterTest {
|
||||
|
||||
private lateinit var bookmarkAdapter: BookmarkAdapter
|
||||
|
||||
private val item = BookmarkNode(
|
||||
BookmarkNodeType.ITEM,
|
||||
"456",
|
||||
"123",
|
||||
0,
|
||||
"Mozilla",
|
||||
"http://mozilla.org",
|
||||
null
|
||||
)
|
||||
|
||||
@Before
|
||||
fun setup() {
|
||||
bookmarkAdapter = spyk(
|
||||
@ -40,33 +31,83 @@ internal class BookmarkAdapterTest {
|
||||
|
||||
@Test
|
||||
fun `update adapter from tree of bookmark nodes, null tree returns empty list`() {
|
||||
val tree = BookmarkNode(
|
||||
BookmarkNodeType.FOLDER, "123", null, 0, "Mobile", null, listOf(
|
||||
item,
|
||||
BookmarkNode(BookmarkNodeType.SEPARATOR, "789", "123", 1, null, null, null),
|
||||
BookmarkNode(
|
||||
BookmarkNodeType.ITEM,
|
||||
"987",
|
||||
"123",
|
||||
2,
|
||||
"Firefox",
|
||||
"https://www.mozilla.org/en-US/firefox/",
|
||||
null
|
||||
)
|
||||
val tree = testFolder("123", "root", listOf(
|
||||
testBookmarkItem("someFolder", "http://mozilla.org"),
|
||||
testSeparator("123"),
|
||||
testBookmarkItem("123", "https://www.mozilla.org/en-US/firefox/")
|
||||
)
|
||||
)
|
||||
bookmarkAdapter.updateData(tree, BookmarkFragmentState.Mode.Normal())
|
||||
bookmarkAdapter.updateData(null, BookmarkFragmentState.Mode.Normal())
|
||||
verifyOrder {
|
||||
bookmarkAdapter.updateData(tree, BookmarkFragmentState.Mode.Normal())
|
||||
bookmarkAdapter.notifyItemRangeInserted(0, 3)
|
||||
bookmarkAdapter.notifyItemRangeInserted(0, 2)
|
||||
bookmarkAdapter.updateData(null, BookmarkFragmentState.Mode.Normal())
|
||||
bookmarkAdapter.notifyItemRangeRemoved(0, 3)
|
||||
bookmarkAdapter.notifyItemRangeRemoved(0, 2)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `update adapter from tree of bookmark nodes, separators are excluded`() {
|
||||
val sep1 = testSeparator("123")
|
||||
val sep2 = testSeparator("123")
|
||||
val item1 = testBookmarkItem("123", "http://mozilla.org")
|
||||
val item2 = testBookmarkItem("123", "https://www.mozilla.org/en-US/firefox/")
|
||||
val folder = testFolder("123", "root", title = "Mobile", children = listOf(item1, sep1, item2, sep2))
|
||||
bookmarkAdapter.updateData(folder, BookmarkFragmentState.Mode.Normal())
|
||||
verifyOrder {
|
||||
bookmarkAdapter.updateData(folder, BookmarkFragmentState.Mode.Normal())
|
||||
bookmarkAdapter.notifyItemRangeInserted(0, 2)
|
||||
}
|
||||
|
||||
assertEquals(2, bookmarkAdapter.itemCount)
|
||||
assertEquals(listOf(item1, item2), bookmarkAdapter.tree)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `update adapter from tree of bookmark nodes, folders are moved to the top`() {
|
||||
val sep1 = testSeparator("123")
|
||||
val item1 = testBookmarkItem("123", "http://mozilla.org")
|
||||
val item2 = testBookmarkItem("123", "https://www.mozilla.org/en-US/firefox/")
|
||||
val item3 = testBookmarkItem("123", "https://www.mozilla.org/en-US/firefox/2")
|
||||
val item4 = testBookmarkItem("125", "https://www.mozilla.org/en-US/firefox/3")
|
||||
val folder2 = testFolder("124", "123", title = "Mobile 2", children = emptyList())
|
||||
val folder3 = testFolder("125", "123", title = "Mobile 3", children = listOf(item4))
|
||||
val folder4 = testFolder("126", "123", title = "Mobile 3", children = emptyList())
|
||||
val folder = testFolder("123", "root", title = "Mobile", children = listOf(
|
||||
folder4, item1, sep1, item2, folder2, folder3, item3
|
||||
))
|
||||
bookmarkAdapter.updateData(folder, BookmarkFragmentState.Mode.Normal())
|
||||
verifyOrder {
|
||||
bookmarkAdapter.updateData(folder, BookmarkFragmentState.Mode.Normal())
|
||||
bookmarkAdapter.notifyItemRangeInserted(0, 6)
|
||||
}
|
||||
|
||||
assertEquals(6, bookmarkAdapter.itemCount)
|
||||
assertEquals(listOf(folder4, folder2, folder3, item1, item2, item3), bookmarkAdapter.tree)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `get item view type for different types of nodes`() {
|
||||
val sep1 = testSeparator("123")
|
||||
val item1 = testBookmarkItem("123", "https://www.mozilla.org/en-US/firefox/")
|
||||
val folder1 = testFolder("124", "123", title = "Mobile 2", children = emptyList())
|
||||
bookmarkAdapter.updateData(
|
||||
testFolder("123", "root", listOf(sep1, item1, folder1)),
|
||||
BookmarkFragmentState.Mode.Normal()
|
||||
)
|
||||
|
||||
assertEquals(2, bookmarkAdapter.itemCount)
|
||||
// item1
|
||||
assertEquals(BookmarkNodeViewHolder.LAYOUT_ID, bookmarkAdapter.getItemViewType(0))
|
||||
// folder1
|
||||
assertEquals(BookmarkNodeViewHolder.LAYOUT_ID, bookmarkAdapter.getItemViewType(1))
|
||||
// sep is dropped during update
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `items are the same if they have the same guids`() {
|
||||
val item = testBookmarkItem("someFolder", "http://mozilla.org")
|
||||
assertTrue(createSingleItemDiffUtil(item, item).areItemsTheSame(0, 0))
|
||||
assertTrue(
|
||||
createSingleItemDiffUtil(
|
||||
@ -84,6 +125,7 @@ internal class BookmarkAdapterTest {
|
||||
|
||||
@Test
|
||||
fun `equal items have same contents unless their selected state changes`() {
|
||||
val item = testBookmarkItem("someFolder", "http://mozilla.org")
|
||||
assertTrue(createSingleItemDiffUtil(item, item).areContentsTheSame(0, 0))
|
||||
assertFalse(
|
||||
createSingleItemDiffUtil(item, item.copy(position = 1)).areContentsTheSame(0, 0)
|
||||
|
||||
@ -16,7 +16,7 @@ import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
|
||||
class UtilsKtTest {
|
||||
@Test
|
||||
fun `friendly root titles`() {
|
||||
val url = testBookmarkItem("folder","http://mozilla.org", "Mozilla")
|
||||
val url = testBookmarkItem("folder", "http://mozilla.org", "Mozilla")
|
||||
assertEquals("Mozilla", friendlyRootTitle(testContext, url))
|
||||
|
||||
val folder = testFolder("456", "folder", null, "Folder")
|
||||
@ -50,7 +50,7 @@ class UtilsKtTest {
|
||||
@Test
|
||||
fun `flatNodeList various cases`() {
|
||||
val url = testBookmarkItem("folder", "http://mozilla.org")
|
||||
val url2 = testBookmarkItem( "folder2", "http://mozilla.org")
|
||||
val url2 = testBookmarkItem("folder2", "http://mozilla.org")
|
||||
assertEquals(emptyList<BookmarkNodeWithDepth>(), url.flatNodeList(null))
|
||||
|
||||
val root = testFolder("root", null, null)
|
||||
@ -99,3 +99,7 @@ internal fun testFolder(guid: String, parentGuid: String?, children: List<Bookma
|
||||
null,
|
||||
children
|
||||
)
|
||||
|
||||
internal fun testSeparator(parentGuid: String) = BookmarkNode(
|
||||
BookmarkNodeType.SEPARATOR, "guid#${Math.random() * 1000}", parentGuid, null, null, null, null
|
||||
)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user