Skip to content

More fine-grained control over data in ListComponent#21

Open
zachbryant wants to merge 5 commits intoYelp:masterfrom
zachbryant:improve_listcomponent_data_flexibility
Open

More fine-grained control over data in ListComponent#21
zachbryant wants to merge 5 commits intoYelp:masterfrom
zachbryant:improve_listcomponent_data_flexibility

Conversation

@zachbryant
Copy link
Copy Markdown

This commit improves the flexibility of ListComponent by enabling more fine-grained updates to the data. The return type of ListComponent.removeData(T data) now returns the int value of the index removed. Additionally a new method to insert data at a certain index in the existing data has been added. The user should be able to replace or re-insert data into the list without re-rendering the entire list to change one item.

@redwarp
Copy link
Copy Markdown
Contributor

redwarp commented Jul 31, 2019

@zachbryant that would be a great addition! It seems though that this change does not take into account the presence or not of dividers. If the ListComponent has separator, then it also displays some grey line between each element. In effet, it displays some extra items, which will impact the index of the item being added. Check https://github.com/Yelp/bento/blob/master/bento/src/main/java/com/yelp/android/bento/components/ListComponent.java#L105

@redwarp redwarp requested a review from dwaxemberg July 31, 2019 13:52
Comment thread bento/src/main/java/com/yelp/android/bento/components/ListComponent.java Outdated
Comment thread bento/src/main/java/com/yelp/android/bento/components/ListComponent.java Outdated
@dwaxemberg
Copy link
Copy Markdown
Member

bump

@zachbryant
Copy link
Copy Markdown
Author

Thanks for the reminder! Things are currently crazy at school but I'll have time to revise my request after Halloween

@redwarp
Copy link
Copy Markdown
Contributor

redwarp commented Feb 5, 2020

@zachbryant bump?

@zachbryant
Copy link
Copy Markdown
Author

zachbryant commented Feb 12, 2020

@zachbryant that would be a great addition! It seems though that this change does not take into account the presence or not of dividers. If the ListComponent has separator, then it also displays some grey line between each element. In effet, it displays some extra items, which will impact the index of the item being added. Check https://github.com/Yelp/bento/blob/master/bento/src/main/java/com/yelp/android/bento/components/ListComponent.java#L105

@redwarp I noticed that no matter what index is given, as long as the item count is correct the whole list re-renders. Not sure if that's intended.. but passing just index seems to be in line with appendData too, unless I'm missing something? Let me know and I'll change it to an adjusted index

Copy link
Copy Markdown
Contributor

@redwarp redwarp left a comment

Choose a reason for hiding this comment

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

Please add tests to ListComponentTest to verify that proper index are updated with and without dividers.

Comment thread .gitignore
@@ -1,5 +1,6 @@
.*
*.iml
.idea
Copy link
Copy Markdown
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

package com.yelp.android.bentosampleapp

import android.os.Bundle
import android.util.Log
Copy link
Copy Markdown
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



private fun addListComponent(controller: ComponentController) {
private fun addListComponent(controller: ComponentController, removable: Boolean = false) {
Copy link
Copy Markdown
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

Comment on lines +102 to +110
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")
Copy link
Copy Markdown
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.

@@ -1,6 +1,7 @@
package com.yelp.android.bentosampleapp
Copy link
Copy Markdown
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...

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
Copy Markdown
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

@redwarp
Copy link
Copy Markdown
Contributor

redwarp commented May 27, 2021

@zachbryant is this still alive?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants