JTAppleCalendar icon indicating copy to clipboard operation
JTAppleCalendar copied to clipboard

func calendar(_ calendar: JTACMonthView, willDisplay cell: JTACDayCell ... not getting called

Open rup2701 opened this issue 6 years ago • 10 comments

(Required) Version Number: Version 8.0.0

Description

Implementing cleanup in willDisplay. But the delegate method is not invoked.

Steps To Reproduce

Implement: JTACMonthViewDelegate#func calendar(_ calendar: JTACMonthView, willDisplay cell: JTACDayCell, forItemAt date: Date, cellState: CellState, indexPath: IndexPath)

Expected Behavior

Cell cleanup.

Additional Context

rup2701 avatar Jul 26 '19 18:07 rup2701

Ok.

Have you moved from version 7.1.7 to version 8? Or, did you start this new project on version 8 directly.

patchthecode avatar Jul 26 '19 18:07 patchthecode

Yep. Everything else is working after moving from 7.1.7 to 8.

I'm pretty sure this method was not working in 7.1.7 either.

rup2701 avatar Jul 26 '19 18:07 rup2701

I'm pretty sure this method was not working in 7.1.7 either.

Yes, because since the beginning i coded it this way. You can see from the lib function below that on the very first line, i do a return if the cell is not dirty. It is only called in very specific times when a cell is dirty.

    public func collectionView(_ collectionView: UICollectionView, willDisplay cell: UICollectionViewCell, forItemAt indexPath: IndexPath) {
        if !pathsToReload.contains(indexPath) { return } // I return here
        pathsToReload.remove(indexPath)
        let cellState: CellState
        if let validCachedCellState = selectedCellData[indexPath]?.cellState {
            cellState = validCachedCellState
        } else {
            cellState = cellStateFromIndexPath(indexPath)
        }
        calendarDelegate!.calendar(self, willDisplay: cell as! JTACDayCell, forItemAt: cellState.date, cellState: cellState, indexPath: indexPath)
    }

But since you are the 1000th person to ask me this question, i am now wondering if i should remove that restriction. I was originally thinking to make the function more efficient (2+ years ago). But now i think that since developers are expecting it to behave exactly like a UICollectionView, maybe i should remove the restriction.

I will give it a re-look to see if efficiency is really impacted. Looking at it with new eyes and 2+ years experience on this. Added the enhancement tag.

But in the mean time, just do what is suggested here and have a shared function between these 2 functions -> https://github.com/patchthecode/JTAppleCalendar/issues/553

patchthecode avatar Jul 26 '19 18:07 patchthecode

Also, are you using Xcode 11 Beta by any chance?

patchthecode avatar Jul 26 '19 18:07 patchthecode

Also, are you using Xcode 11 Beta by any chance?

Nope. 10.3 GA.

rup2701 avatar Jul 26 '19 18:07 rup2701

I'm pretty sure this method was not working in 7.1.7 either.

Yes, because since the beginning i coded it this way. You can see from the lib function below that on the very first line, i do a return if the cell is not dirty. It is only called in very specific times when a cell is dirty.

    public func collectionView(_ collectionView: UICollectionView, willDisplay cell: UICollectionViewCell, forItemAt indexPath: IndexPath) {
        if !pathsToReload.contains(indexPath) { return } // I return here
        pathsToReload.remove(indexPath)
        let cellState: CellState
        if let validCachedCellState = selectedCellData[indexPath]?.cellState {
            cellState = validCachedCellState
        } else {
            cellState = cellStateFromIndexPath(indexPath)
        }
        calendarDelegate!.calendar(self, willDisplay: cell as! JTACDayCell, forItemAt: cellState.date, cellState: cellState, indexPath: indexPath)
    }

But since you are the 1000th person to ask me this question, i am now wondering if i should remove that restriction. I was originally thinking to make the function more efficient (2+ years ago). But now i think that since developers are expecting it to behave exactly like a UICollectionView, maybe i should remove the restriction.

I will give it a re-look to see if efficiency is really impacted. Looking at it with new eyes and 2+ years experience on this. Added the enhancement tag.

Fair question. I did not dip in your code. Because as a wrapper I expected it to behave like a normal collectionView.

If any efficiency I was thinking it should be in my impl. For example, I'm marking the current day label as red. But since cells are recycled I get many different days marked as red. Hence the check in willDisplay.

image

rup2701 avatar Jul 26 '19 19:07 rup2701

For now I have just put the cell dequeue and config code in func cellForItemAt.

rup2701 avatar Jul 26 '19 19:07 rup2701

Hi JT,

Am I reading #553 correctly that implementing the shared code will allow willDisplay to be called?

rup2701 avatar Jul 30 '19 15:07 rup2701

no. I t just means that the code that is in the willDisplay and the cellForItem functions should be the same. Therefore, to reduce code duplication, just make them call a shared function.

If you want willDisplay to work as a regular UICollectionView, then wait a bit on the update.

patchthecode avatar Jul 30 '19 17:07 patchthecode

Ok. Thanks.

rup2701 avatar Jul 30 '19 17:07 rup2701