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

Support for data binding of adapter's data set #217

Closed
dozd opened this issue Nov 2, 2016 · 9 comments
Closed

Support for data binding of adapter's data set #217

dozd opened this issue Nov 2, 2016 · 9 comments

Comments

@dozd
Copy link

dozd commented Nov 2, 2016

Hello, first of all, your FlexibleAdapter rocks :-).

As I'm big fan of Android's data binding - support for data binding of adapter's data set would be great. Imagine you have ObservableArrayList<IFlexible> and you don't have to call updateDataSet when the data changes. What do you think about it? I could try to create PR if interested.

View holders could be bound via createViewHolder and bindViewHolder and it's more than sufficient.

@davideas
Copy link
Owner

davideas commented Nov 2, 2016

@dozd, thank you for your nice message!

It could be a good extension, maybe if it doesn't take too much effort for you, you can create the PR so I can check better what do you mean exactly when you say that updateDataSet() won't be called.

@dozd
Copy link
Author

dozd commented Nov 3, 2016

@davideas, I've commited PoC to demonstrate what I mean.

In sample app, look at FragmentDataBindingHeadersSections, mainly performFabAction where I'm modifying my list of items and adapter is updated automatically.

Have in mind I didn't tune performance or best way how to implement it as I didn't put much effort into it :). Just want know what do you think of it. I took part of code (callback) from binding-collection-adapter that I'm familiar with and using on one of my project.

@davideas
Copy link
Owner

davideas commented Nov 4, 2016

@dozd, I studied the changes you have done, so databinding obliges us to specify models and data classes directly in the layout (presentation view) and allow us to write some logic too.
I have read this article http://www.anwarshahriar.me/scratching-the-surface-of-androids-new-data-binding-library-and-retrofit-part-2/ and I find it much more complicated for us to bind data.
In the end it avoids us to not call view.setText().

Also this article, https://www.bignerdranch.com/blog/descent-into-databinding/ says that we save to specify the name of the view in the code, but we put our classes in the view.

I don't like this mixture of code in the views, it is always better to separate views and classes that manage those views, business layer from presentation layer.

I wait your consideration as well.

@dozd
Copy link
Author

dozd commented Nov 4, 2016

It's all about MVP (or C). As model-view is considered to be one of the most suitable design pattern for UI applications, data binding is considered its upgrade. In MV word, all of your UI is defined strictly by the model class. Changes to the model must be reflected in the UI and vice versa. The model defines the state of the UI. And there must be connection between model and view. It's either presenter or controller (or is partially replaced by data binding).

If you follow MV pattern in Android world, after changes in the model you must call from your presenter (or controller) the view (fragment and activity). In fragment, you find proper ui elements (findById or injection) and change them. After some actions in UI, you call fragment callback, then presenter (or controller) and then you change your model accordingly. This is true and good business / view separation to follow. Fragment is still the view layer.

Data binding simplifies this process very much. You don't need to call fragment or activity from your presenter, you don't need to call UI changes in fragment (you don't need view elements injection), you don't need to have some callbacks in fragment and (in case of two-way binding) you don't need to have callbacks in some cases at all.

And as the TextView is changed automatically when I change string field in my model, as this field in the model is changed after user edit that text in EditText, I also want RecyclerView be automatically updated after i change list of items in my model and also my model is updated when user makes swipe to delete or another change in the UI. That's where android databinding can shine and really make it simple.

In the sample app (and my committed classes), there is no true business / view separation so the power of data binding is hidden. It's just show case, that data binding could work with your library.

@roee88
Copy link

roee88 commented Nov 28, 2016

+1 for that.
I've just reviewed FlexibleAdapter before using it in my application.
Great library, but the lack of databinding support stood out.

@davideas davideas modified the milestone: 5.0.0-rc2 Nov 28, 2016
@davideas
Copy link
Owner

davideas commented Feb 14, 2017

Can somebody check the current branch extensions for databinding extension?
Could you please identify if something is wrong or could be done in different way?

Note:
I added an item in the demoApp in the GridView to easily enter in the example.
FAB button will create changes to the list (check performFabAction() in FragmentDataBinding.

Modifications:

  • I added a copy of items, in BindingAdapters#L19, otherwise internal changes will make conflicts on the observable.
  • mItems is not present anymore in BindingFlexibleAdapter: the internal synchronization engine will calculate the changes when updateDataSet is invoked: BindingFlexibleAdapter#L38, normally if animate = false notifyDataSetChanges is always invoked.
  • Where can we add boolean parameter for animate=true for updateDataSet?

@IHNEL
Copy link

IHNEL commented Feb 26, 2017

+1 for support data-binding.

@davideas
Copy link
Owner

Still waiting somebody that wants to test and verify this new improvement.

@davideas
Copy link
Owner

davideas commented May 3, 2017

A first beta version of this extension is now available.
For the moment only in the demoApp in current_snapshot branch is shown how to use this extension. Setup is in the Readme of that branch too.

@davideas davideas closed this as completed May 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants