enpassio / Databinding

Various examples on databinding which can help not only understand how to get started with it but it also shows how this can be used in complex and real use cases
Apache License 2.0
26 stars 14 forks source link

Dev databinding twoway #22

Closed OyaCanli closed 5 years ago

OyaCanli commented 5 years ago

@rajtheinnovator Should I add a warning for unsaved changes? Or just don't care about those details since this is just a sample. What do you think?

OyaCanli commented 5 years ago

I'll also change the attachCategories helper method. It's a bit ugly to first add and then delete last comma. I found a better algorithm using Iterators. And in Kotlin there is joinToString method which deals with that.

rajtheinnovator commented 5 years ago

@OyaCanli I think and then it'd be nice to add warning wherever needed as that'll help a developer understand them better. I'll be making another comment in the other PR

OyaCanli commented 5 years ago

We are talking about different samples. You are talking about kotlin sample. I'll check it out.

rajtheinnovator commented 5 years ago

Yeah, I was talking about another sample and have just left a comment in respective PR. Please check it and let me know what you see there. For this PR, I'll try running the project and if it goes well, I'll simply make the merge

OyaCanli commented 5 years ago

Ok. I'll make some modifications in this one as well. (What we discussed above and may be a few other minor modifications)

rajtheinnovator commented 5 years ago

Sure thing. For now, I'm reviewing the code and checking it in details by building on my machine

OyaCanli commented 5 years ago

@rajtheinnovator In fact, this project has bugs. The map of checkboxes doesn't seem to work two-way. I'll try to fix this as soon as possible. May be I can change strategy and use a list instead of map..

rajtheinnovator commented 5 years ago

Sure thing Oya. I tried the code but didn't notice it. Good that you caught it 👍

On Tue, Feb 19, 2019, 7:43 PM Oya Canli <notifications@github.com wrote:

@rajtheinnovator https://github.com/rajtheinnovator In fact, this project has bugs. The map of checkboxes doesn't seem to work two-way. I'll try to fix this as soon as possible. May be I can change strategy and use a list instead of map..

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/enpassio/Databinding/pull/22#issuecomment-465143371, or mute the thread https://github.com/notifications/unsubscribe-auth/AICiSvjTPwv9Quxx5vai-Pf5-MqLhDekks5vPAZ8gaJpZM4ah9sj .

OyaCanli commented 5 years ago

Well, weird, but in the end, it seems that maps were working fine two-way. I don't know why it was not working this afternoon. May be I temporarily broke something while modifing.. Anyway, that part works.. But there is another problem, due to the fact that I'm initializing chosenProduct in onActivityCreated and this is called at each resume or after rotation. This can be fixed by using a seperate viewmodel for second fragment. That way initializations will be made once in the constructor. Using shared viewmodel between list and detail fragments, with setItem/getItem model is very practical, but in some cases it creates problems.

rajtheinnovator commented 5 years ago

Okay. Good to know that. Thanks for checking again @OyaCanli . And for the idea of separate viewModel, I think we should go for that. If the fragment is having separate logic from another fragments(which generally the case is), we should go for separate ViewModel, from what I've seen some experienced developer doing. However they do mention that we should be careful of lifecycle of fragments/viewModel whenever working on multiple ViewModel inside same activity.