EurekaCommunity / SplitRow

A row for Eureka to put two rows side by side into the same UITableViewCell
MIT License
56 stars 27 forks source link

Values are getting cleared if one of the row types is ActionSheet #16

Closed kamerc closed 6 years ago

kamerc commented 6 years ago

I have 3 types of SplitRows in my app: SplitRow<ActionSheetRow, EmailRow> SplitRow<ActionSheetRow, PhoneRow> SplitRow<TextRow, TextRow>

I am seeing an issue with the first 2. The scenario is that the user opens an edit modal and the SplitRows with existing values are loaded just fine (all the types and values are populated for email and phone). However, if I go to change the value of the type within action sheet (aka I change the left side), the value on the right side clears. If I had changed the value on the right initially instead, the left side clears. If I override the code to not call subscribe for onChange at all, it works perfectly.

I am having the same issue when I go to move focus off of this row entirely. When I do, both values (left and right) are cleared. This particular issue wasn't happening until I got your latest update. If I override the code to not call subscribe for onCellHighlightChanged, it works perfectly.

I have not seen this weird behavior when both types are TextRow, only when both contain an ActionSheet. I have not tested PushRow but I suspect it may be similar.

So I have got around my issue by creating my own custom class but it's not an ideal solution. Can you tell me what the purpose of the code for onChange and onCellHighlighted is? Since it works perfect without it, I haven't figured out what the purpose of the code is. I am NOT using any custom tag names or anything like that.

This is my simple custom class that I'm using now to get it working as I would expect in all cases:

import Foundation
import Eureka
import SplitRow

public final class OrionSplitRow<L: RowType, R: RowType>: _SplitRow<L,R>, RowType where L: BaseRow, R: BaseRow {
    override public func subscribe<T>(onChange row: T) where T : BaseRow, T : RowType {
        //Do nothing
    }
    override public func subscribe<T>(onCellHighlightChanged row: T) where T : BaseRow, T : RowType {
        //Do nothing
    }
}
marbetschar commented 6 years ago

@kamerc thanks for the detailed break down of the issue. I'll need to dig into it, but for me it sounds the cause of the issue lies somewhere around setting/getting the value of the entire SplitRow together with the logic to detect potential duplicates of firing the events (somewhere near here).

That said, the onChange and onCellHighlightChanged of the embedded rows are subscribed to in order to forward needed information to the enclosing SplitRow - which then in turn sets it's own value.

Maybe we can simply circumvent this by just use a computed getter/setter for the SplitRows value - seems like this makes things easier anyways. However, I have to check this first and run some tests.

Thanks for pointing this out!

kamerc commented 6 years ago

I also wanted to add that the weird behavior seemed to only happen the very first time I change the ActionSheet value. Once I have changed it at least once, it didn't act odd anymore. I put some debugger lines in and that very first time I changed an action sheet value I saw it called onChange for left, then onChange for right then onChange for left again. Once it has been changed that very first time, it always correctly just calls onChange for left like it should. Not sure why the additional onChanges are getting called but that is likely what is causing the values to clear out.

marbetschar commented 6 years ago

@kamerc how do you exactly populate the rows with the existing values when opening the edit modal? Do you have some sample code showing this?

marbetschar commented 6 years ago

@kamerc I think I was able to reproduce the bug and pushed a potential fix in issues/16 branch. Can you please give it a test drive and verify if the issue is solved?

kamerc commented 6 years ago

I tested it out and it fixed my issue. Thanks!