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

StepperRow cell artifact when using Eureka v5.1.0 #39

Closed sedwo closed 4 years ago

sedwo commented 4 years ago

Similar to the Eureka issue listed here: https://github.com/xmartlabs/Eureka/issues/1910

Except SplitRow seems to create a dual stepper control.

eg.

let signNumberQuantityRow = SplitRow<TextFloatLabelRow, StepperRow>() { row in

image

sedwo commented 4 years ago

Hi @mats-claassen,

I've replicated the issue here using Xcode 11, iOS 13 simulator.

As I work to debug it, would you happen to have any insight as to why this UI artifact occurs?

SplitRow-Stepper.zip

fyi: Set Swift Language Version = "Swift 4" within Build Settings for FloatLabelRow pod

sedwo commented 4 years ago

@mats-claassen I tested a few Eureka versions and this artifact seems to occur only with v5.0.1

mats-claassen commented 4 years ago

I think this is related to the Eureka issue you mentioned. There is a fix waiting to be merged. Let's see if the issue here is still there after with that fix

sedwo commented 4 years ago

@mats-claassen v5.1.0 of Eureka still has this issue. :/

sedwo commented 4 years ago

@kamerc Do you exhibit this issue here?

kamerc commented 4 years ago

@sedwo I am not using StepperRow in my own code so I don't see this issue.

sedwo commented 4 years ago

@mats-claassen bump. 😬

mats-claassen commented 4 years ago

Hi, I have looked into this. Issue is that the setup function of the cells inside SplitRow is called twice, which in the case of the StepperRow ends in two steppers being added but it might lead to other issues with other rows.

Issue is in SplitRowCell here (* edited class name). By accessing the cell of a row (the first time) the cell is being initialized and setup is called. setup is called manually again later inside the setup of the inner tableView.

Possible fix is to remove line 36 (row.baseCell.setup()) from SplitRowCellTableView

marbetschar commented 4 years ago

@mats-claassen is it guaranteed for all row types, that the row's setup function is always called before SplitRow executes it?

marbetschar commented 4 years ago

We surely can add an exception in SplitRow for StepperRow as long as it’s shipped with Eureka core. But not quite sure if this is the best approach...?

mats-claassen commented 4 years ago

Sorry I misspelled the class name above. Corrected it now. Issue is in SplitRowCell. I do think this issue affects all rows. However it is possible that some rows work fine with setup being called twice even though by design it is not correct.

I have not tested the fix I suggested but I am quite positive about it

marbetschar commented 4 years ago

@mats-claassen thanks for the hint! I've added the following fix in master branch:

if !(row is _StepperRow) {
    row.baseCell.setup()
}

What I've seen it works. Can you guys verify everything behaves as it should? Will do a new release after confirmation.

EDIT: As the issue only arises on StepperRow right now, I'd prefer fixing this issue explicitly for this one row type to avoid any potential side effects. If the issue arises again for another row type, we should definately look further into this and find a way to make sure setup() is called only once.

sedwo commented 4 years ago

Thank you @marbetschar, your change works. 👍 Tested with Eureka v5.2.0.