Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More fine-grained control over data in ListComponent #21

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
.*
*.iml
.idea
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be there


gradle.log
local.properties
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.yelp.android.bentosampleapp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be aware that the ListViewActivity was created to verify that list views display stuff properly. There is no link between the ListViewActivity and the ListComponent. Bento is compatible with both ListViews and RecyclerViews, ViewPagers...


import android.os.Bundle
import android.util.Log
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be there, Log is not used

import android.view.Menu
import android.view.MenuItem
import android.widget.ArrayAdapter
Expand Down Expand Up @@ -44,6 +45,7 @@ class ListViewActivity : AppCompatActivity() {
addCarouselComponent(controller)
addArrayAdapterComponent(controller)
addAnimatedComponent(controller)
addListComponentRemovableItems(controller)
}

override fun onCreateOptionsMenu(menu: Menu): Boolean {
Expand Down Expand Up @@ -79,8 +81,7 @@ class ListViewActivity : AppCompatActivity() {
controller.addComponent(simpleComponent)
}


private fun addListComponent(controller: ComponentController) {
private fun addListComponent(controller: ComponentController, removable: Boolean = false) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the removable flag is not used

with(ListComponent(null,
ListComponentExampleViewHolder::class.java)) {
setStartGap(50)
Expand All @@ -90,6 +91,26 @@ class ListViewActivity : AppCompatActivity() {
}
}

private fun addListComponentRemovableItems(controller: ComponentController) {
val component = ListComponent(null, ListComponentExampleViewHolder::class.java)
with(component) {
setStartGap(50)
setData(listOf("List element 1"))
toggleDivider(true)
controller.addComponent(this)

listOf(1, 1, 1, 2, 4).forEachIndexed { index, i -> insertData(i, "List element ${index + 2}") }
insertData(0, "List element First")
insertData(0, "List element First")
insertData(1 + component.count / 2, "List element Last")
removeData("List element First")
val index = removeData("List element 1")
insertData(index, "List element 1")
removeData("List element 2")
removeData("List element nonexistent")
Comment on lines +102 to +110
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a really useful code: you are adding and removing items even before the component would be displayed on screen. You might want to consider instead first displaying a list of items, and adding a button somewhere to try the scenario where you remove items dynamically.

}
}

private fun addArrayAdapterComponent(controller: ComponentController) {
val arrayAdapter = ArrayAdapter<String>(this, android.R.layout.simple_list_item_1,
android.R.id.text1, (1 until 42).map { "ArrayAdapter element $it" })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,17 +86,31 @@ public void appendData(@NonNull List<T> data) {
}

/**
* Removes the provided data items from the list.
* Inserts a single list item at the specified index in the list.
*
* @param index The index used to insert the given data in the list.
* @param data The list item to be inserted.
*/
public void insertData(int index, @NonNull T data) {
mData.add(index, data);
// Update 2 items if dividers are showing.
notifyItemRangeInserted(index, mShouldShowDivider ? 2 : 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if divider are visible, you probably need to multiply index by 2, otherwise you will have some surprises

}

/**
* Removes the first occurrence of the specified data item from the list.
*
* @param data The data item to remove from the list.
* @return The index that the data occupied. Returns -1 if nothing was removed.
*/
public void removeData(@NonNull T data) {
public int removeData(@NonNull T data) {
int index = mData.indexOf(data);
// Check if the object indeed is in the list.
if (index != -1) {
mData.remove(index);
notifyItemRangeRemoved(getRemoveIndexStart(index), getRemoveItemCount());
}
return index;
}

/**
Expand Down