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

onCellUnHighlight and row.reload #761

Open
allaire opened this issue Oct 26, 2016 · 8 comments
Open

onCellUnHighlight and row.reload #761

allaire opened this issue Oct 26, 2016 · 8 comments

Comments

@allaire
Copy link
Contributor

allaire commented Oct 26, 2016

Hi,

it seems that when I try to call row.reload() in onCellUnHighlight (because I want my IntRow to get refreshed and pick the new value of displayValueFor I declared in setup), it does not reload the cell with the good format. Same thing with row.updateCell().

However, when I scroll and the cell is off the screen, and scroll back to it, then it's formatted correctly.

The only way to fix it is to add a bit of delay before calling row.reload() in onCellUnHighlight (dispatch_async(dispatch_get_main_queue()) also works) :

        unpaidBreakRow = IntRow("unpaidBreakRow") {
            $0.title = t("timesheet.time_entry.unpaid_break")
            $0.value = 0
            $0.disabled = Condition(booleanLiteral: (self.mode == Mode.Show))

            $0.displayValueFor = { (value: Int?) in
                if let value = value {
                    return t("general.x_min", params: [String(value)])
                } else {
                    return nil
                }
            }
        }.onChange { (row) in
            print("BB")
            self.validateFields()
        }.onCellHighlight { (cell, row) in
            if row.value == 0 {
                row.value = nil
            }
        }.onCellUnHighlight { (cell, row) in
            print("AA")
            row.reload() // Not working
            row.updateCell() // Not working
//            delay(0) { // Works (or dispatch_async(dispatch_get_main_queue()) {)
//                row.reload()
//            }
        }

Calling self.tableView?.reloadData() also seems to work.

When I use row.reload(.Fade), the cell disappears completely.

Is this the expected behaviour?

@allaire
Copy link
Contributor Author

allaire commented Oct 26, 2016

On another topic, it's still a bit unclear for me when to use row.reload() or row.updateCell(). I think this is something we should clarify in the README.

@allaire
Copy link
Contributor Author

allaire commented Oct 26, 2016

@mats-claassen @mtnbarreto If this is a bug, don't hesitate to give me some hints, I can try to fix it myself and submit a PR 👍

@mtnbarreto
Copy link
Member

mtnbarreto commented Oct 26, 2016

I would not say this is a bug.. We will have to add some clarification on the FAQ readme section.

basically row.reload() reloads the table view cell ;), which means tableView.reloadRows (func reloadRows(at indexPaths: [IndexPath], with animation: UITableViewRowAnimation)) method will be called with the correspond index.

According apple documentation:

Reloading a row causes the table view to ask its data source for a new cell for that row. The table animates that new cell in as it animates the old row out. Call this method if you want to alert the user that the value of a cell is changing. If, however, notifying the user is not important—that is, you just want to change the value that a cell is displaying—you can get the cell for a particular row and set its new value. When this method is called in an animation block defined by the beginUpdates() and endUpdates() methods, it behaves similarly to deleteRows(at:with:). The indexes that UITableView passes to the method are specified in the state of the table view prior to any updates. This happens regardless of ordering of the insertion, deletion, and reloading method calls within the animation block.

On the other hand row.updateCell() just refresh the cell view calling all relevant Eureka callbacks. I think is more suitable to update the cell's UI when row.value changes.

New eureka version calls updateCell when a cell gets the first responder as the following code shows.

    /**
    Called when a cell becomes first responder
    */
    public final func beginEditing<T:Equatable>(of cell: Cell<T>) {
        cell.row.isHighlighted = true
        cell.row.updateCell()
        RowDefaults.onCellHighlightChanged["\(type(of: cell.row!))"]?(cell, cell.row)
        cell.row.callbackOnCellHighlightChanged?()
        ...
        ...
        }
    }

If you only want to update the cell, you should use row.updateCell.
I really don't know what's going wrong and why we need the dispatch_async.

@allaire
Copy link
Contributor Author

allaire commented Oct 26, 2016

@mtnbarreto Thanks for the explanation.

Although I get the same issue with row.updateCell in my onCellUnHighlight, I still need the dispatch_async.

If tomorrow I can duplicate this behaviour in the exemple project of Eureka, would you take a look?

@mtnbarreto
Copy link
Member

sure! do it in the master branch code please.

regards

@allaire
Copy link
Contributor Author

allaire commented Oct 27, 2016

@mtnbarreto I just reproduced the async issue in the master branch, here's the code: https://github.com/allaire/Eureka

@mtnbarreto
Copy link
Member

mtnbarreto commented Oct 27, 2016

I looked at the code.

Basically the issue comes from open func textFieldDidBeginEditing(_ textField: UITextField) that invokes onCellHighlightChanged before setting textField text.

open func textFieldDidEndEditing(_ textField: UITextField) {
        formViewController()?.endEditing(of: self) // --> it invokes `onCellHighlightChanged`
        formViewController()?.textInputDidEndEditing(textField, cell: self)
        textFieldDidChange(textField)
        textField.text = displayValue(useFormatter: (row as? FormatterConformance)?.formatter != nil) -> overrides the text field text 
}

Based on that, there is no UIKit nor thread/race condition issue.

By using the dispatch_async you are setting up textField.text after textFieldDidEndEditing finishes so it works.

Let me see if we can change the sentences order to avoid this error.

@allaire
Copy link
Contributor Author

allaire commented Oct 27, 2016

@mtnbarreto Good catch! Let me know how this goes :)

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

2 participants