This component is a combination of Tab Layout and View Pager and hence leverages the features of both

Issue

The CarouselComponent shares the same view pool as the main RecyclerView, which is great.

But when the CarouselComponent is scrolled out of the page, it will not release it's own Components, until it is itself recycled.

Having a few item not recycled is probably not a big deal, but it has another side effect: its components's onViewDetachedFromWindow callbacks are not called, because they are still technically inside their parent, the carousel's recyclerview.

In this screenshot, I have the case where my ComponentController like that:

• ComponentController:
  • ListComponent
    • CardComponent
    • CardComponent
    • …
  • CarouselComponent
    • CardComponent
    • CardComponent
    • …
  • ListComponent
    • CardComponent
    • CardComponent
    • …

As this contains only 1 CarouselComponent, this carousel component will never be recycled, as no other item needs to reuse its ViewHolder.

Proposed solution

I propose, in CarouselComponentViewHolder#onViewDetachedFromWindow, to clear the Controller.

As there is no guaranties that the bind method will be called again, we then need to add a snippet of code in the onViewAttachedToWindow method to restore the controller's element if we detect it was cleared before. And if that is the case, then we need, once more, to scroll to the correct position.

Wondering if we can add a way to configure cache size and initial prefetch count in the recycler view component controller

When using a ViewPagerComponent within another component controller, scrolling from the view pager does not scroll it's parent component controller. We should make the parent component controller take precedent.

When a Carousel is scrolled away from view in a recycler view, it's nested components are not notified that they are becoming invisible.

When the carousel get's back into view, its nested component then don't get notified that they are becoming visible again.

This behavior prevents per card actions to be generated

Right now is not possible to have access to LayoutManager and make changes to its scrollable methods. We could expose scrolling options on the constructor or allow RecyclerViewComponentController to receive a LayoutManager making it more flexible to other use case.

I didn't went through all the controllers but I think either exposing LayoutManager properties or itself entirely are good options to increase flexibility for the recyclerview and decouple RecyclerViewComponentController from LayoutManager

Some dependencies, like powermock, are declared into GlobalDependencies.kt but never used.
That should be cleaned

When a ListComponent has dividers, Bento will send an onItemVisible() and onItemNotVisible() for the dividers as well as the list items.

This causes confusion to consumers of the ListComponent that expect to be able to use the provided index as the index of the items.

We should have an alternative set of methods for ListComponent that only pertain to the items and not the dividers.

When testing a page with a gap in the header, I noticed that the page crashed. I believe this is because it doesn't know how to deal with the span of a gap. I'm not quite sure how this works right now, but we should make the method look more like this:

...
public int getSpanSize(int position) {	           
+   if (hasGap(position)) {	
+       return getNumberLanes();	
+   }	
    RangedValue<Component> rangedValue = mComponentAccordionList.rangedValueAt(position);
...

Seems like bento has not been updated to drop jcenter yet

There is a crash if the user attempts to reorder a ListComponent with dividers. This is because the ListComponent implements onItemsMoved and automatically moves the data in mData. The size of mData will always be (about) half of the total size of the component because of the dividers.

I believe the solution to this will be checking if dividers are enabled, and if so, dividing them by 2 when reordering the data.

I would like this repo to adopt conventional commits as a way to enforce that our changelog is consistent and can be used to automatically generate changelogs when we are release new versions.

https://www.conventionalcommits.org/en/v1.0.0/

A quick overview:

The commit contains the following structural elements, to communicate intent to the consumers of your library:

fix: a commit of the type fix patches a bug in your codebase (this correlates with PATCH in semantic versioning).
feat: a commit of the type feat introduces a new feature to the codebase (this correlates with MINOR in semantic versioning).
BREAKING CHANGE: a commit that has a footer BREAKING CHANGE:, or appends a ! after the type/scope, introduces a breaking API change (correlating with MAJOR in semantic versioning). A BREAKING CHANGE can be part of commits of any type.
types other than fix: and feat: are allowed, for example @commitlint/config-conventional (based on the the Angular convention) recommends build:, chore:, ci:, docs:, style:, refactor:, perf:, test:, and others.
footers other than BREAKING CHANGE: may be provided and follow a convention similar to git trailer format.

The benefits of this kind of format is that thanks to its structure, tooling exist around it to automatically generate change logs in a readable and structured form based on the commit messages. See https://www.conventionalcommits.org/en/v1.0.0/#tooling-for-conventional-commits

Also, it is possible to add some github actions like https://github.com/outillage/commitsar directly to the project to make sure that every commit follows the standard.

I was looking through the ReorderListActivity to understand how to implement drag and drop and was unable to understand how to update data inside of a ListComponent.

From the example:

val handleComponent = ListComponent(this, ReorderViewHolder::class.java, 2)
handleComponent.setIsReorderable(true)
handleComponent.toggleDivider(false)
handleComponent.setData((0..10).map { 'A'.plus(it).toString() })
handleComponent.setOnItemMovedCallback(object : OnItemMovedCallback<String> {
    override fun onItemMoved(oldIndex: Int, newIndex: Int) {
        Log.i("Reordered", "Item at $oldIndex moved to $newIndex")
    }
})
val componentGroup = ComponentGroup().apply {
    addComponent(LabeledComponent("Drag handle to reorder"))
    addComponent(handleComponent)
}
componentController.addComponent(componentGroup)

This works super great and allows for effectively dragging the items around. However, in my use case I would like to show the index of the current cell to help provide context around which position you are now placing the cells into. My data therefore is a simple data class something like this:

data class CellData {
    var index: Int.
    var text: String
}

What my instinct was to do is update the data in the array, call notifyItemRangeChanged and call it good. But, since the data appears to be copied i don't have a reference to the actual data anymore. As a result, I found myself just calling list.setData(UPDATED_DATA) to get the desired updates however that was causing items to flash. This then led me to removing animations on changes like so.

(recyclerView.itemAnimator as? SimpleItemAnimator)?.supportsChangeAnimations = false 

I think ultimately it would be nice to have either a updateItemAt or allow for setData to take a notify boolean. That way you can update the data behind the scenes and then trigger the update on the appropriate cells.

It is really a RecyclerView issue: when we insert an item in a RecyclerView at position 0, the RecyclerView does not scroll up or down.
This is of course wanted behaviour in most cases, except for one: when you are at scroll offset 0, it would be better to stay at scroll offset 0 and display the new item.

The use case that I have is the following. You are on the home page of your app. And you query some asynchronous service to query for some notifications banners.
It's like, "hot informations" that are displayed on top of the page.
Your asynchronous service returns info, you create a banner, and insert it at index 0. You will actually need to manually scroll up to see it.

There are a bunch of failing tests in master. Looks like we aren't running all tests as part of CI and have allowed a bunch to break.

Proposal

It seems the bento library is getting clogged by classes that correspond to a specific use case.

For readability, we could split the library in two, and publish two artifacts.

One would be the core library, which only contains what is necessary for bento to work: small, concise, would contain the content of the core package.

Anything else, like the paginationlistcomponent, nested component, list component... could be splitted in a different library.

Why?

It feels that a bunch of the component that we include extra are quite opinionated:

• The list component by default displays separators.
• The SimpleComponent takes a presenter but no items? (It will return the presenter, but always null for the item)
• ... and more

These opinion are diluating the simplicity of Bento.

We currently do not have a strict code formatting discipline.
I propose we use ktlint as it's a well documented, well maintainted tool.

There is several option to enforce ktlint:

• Add a github action like https://github.com/ScaCap/action-ktlint which will check if the modified files pass validation -> only drawback is that we are not so sure about this action, if it will stay maintained. We might have to help?
• We can use one of the ktlint gradle plugin together with a Travis config that runs the check, like https://github.com/kafumi/android-danger-ktlint-sample/blob/master/.travis.yml (gradle plugins are documented on the ktlint repo, see https://github.com/pinterest/ktlint#-with-gradle)
• We can have a bot that once in a while runs ktlint on every file, to auto-format what can be formatted such as this script: https://gist.github.com/wasabeef/60113a9d4dec64ad71a901c089aae8b5
• We can write our own script to do our own stuff.

In Android, drag and drop reordering is a standard: https://material.io/design/components/lists.html#behavior

In general, there are two ways to initialize a reorder. Either you can press and hold on a list item, then move it to the desired location and release, or you can touch a "handle" on it that allows you to move it without long pressing.

Ideally, this should be enabled in Bento. Doing so would be relatively simple because the RecyclerView already supports these actions.

In the ComponentVisibilityListener, the method that tries to notify nested components does not actually verifies that these components are visible.

We need to add a guard to firstVisible and lastVisible to verify they are not a NO_POSITION

As the migration to Android X is done, do we still need to keep the jetifier folder?

We should keep a better track of what's new in bento.

Would be useful for adding borders to components or having dynamic inner components inside outer ones.

As I Known that ViewHolder is for display a item ui of entity
so,Why the **ViewHolder had needed a Presenter?
why do not retrocede the business logic to outside(ex:activity) and ViewHolder just handle the event(click、long click)?
Sincerely ask for advice.

when I append 3 datas in ListComponent
it just can show one new item
and then I append 3 datas again
it show the last 2 item , the new data is gone
and repeat
btw, I try it in the demo

Coulde the ViewPager2ComponentController not only support place A viewgroup but A fragment?
Viewpager2、 Fragment、 FragmentStateAdapter，U know what i mean, right?

Currently, CarouselComponent only allows for adding components. It would be useful to have remove() and clear() functionality as well.

In a use case where we create a list component with more than 1 lane, enable dividers and specify a divider view, components will only be spaced by the divider vertically. The divider won't show between lanes.

Maybe add an option to set flags for, HORIZONTAL, VERTICAL, BOTH for where we want the divider view to show.
We could also specify separate divider views for horizontal and vertical.

A few files seem oddly Yelp Specific:

• OWNERS
• Jenkinsfile

They could probably be removed?