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

initDateDidChange() and scrolling layout malfunction in 2018-11-29 update. #40

Closed
theoadahl opened this issue Dec 13, 2018 · 9 comments
Closed

Comments

@theoadahl
Copy link

theoadahl commented Dec 13, 2018

Introduction

Hello, i've been using this library since end of august this year and loving every moment of it. However after updating the library 15 days ago (when 16cf27b ) was merged, i got the following problems which forced me to reverse to the version i had before (whatever version that running 23 nov 2018).

Context

  • I use manual approach to install the library: dragging and dropping the files.
  • Below you will find my subclass of JZLongPressWeekView that I use as well as the setup method for the calendar in my view controller. I get the same issue even if i change back to using the supplementary views that come with the library!
  • Summary of version: Does not happen when 16cf27b has not been merged. I.e. versions before 28th of november.
  • Note: first load up of the week view works perfectly and displays events and layout as it should.

Actual issue

  • I've noticed through print statements (and actual non-loading of new events) that initDateDidChange(_ weekView: JZBaseWeekView, initDate: Date) does not get called when i scroll to change page.
  • When i scroll to the next page (into future dates), then scroll back to the first, the alldayheader is not populated with events. And a peculiar behavior happens: If i then scroll further back (into the past week) there is just a white background showing on that week and when i let go of the scroll it sort of jumps and loads up the first page again with the alldayheader populated. I hope this is clearly written...

Code

JZLongPressWeekView subclass:

final class WeekCalendarViewModel: JZLongPressWeekView {
    private let eventInfoView        = CNWeekViewEventInfoView(frame: .zero)
    private var eventInfoXConstraint = NSLayoutConstraint()

    override func registerViewClasses() {
        self.collectionView.register(CNWeekViewCell.self, forCellWithReuseIdentifier: CNCellAndHeaderID.weekCalendarCell)
        self.collectionView.registerSupplimentaryViews([CNDateHeader.self, CNMonthHeader.self, JZRowHeader.self, CNAllDayHeader.self])
        flowLayout.registerDecorationViews([CNDateHeaderBackground.self, JZRowHeaderBackground.self,
                                        JZAllDayHeaderBackground.self, JZAllDayCorner.self])
        flowLayout.register(JZGridLine.self, forDecorationViewOfKind: JZDecorationViewKinds.verticalGridline)
        flowLayout.register(JZGridLine.self, forDecorationViewOfKind:     JZDecorationViewKinds.horizontalGridline)
        collectionView.alwaysBounceVertical = true
        collectionView.bounces = true
    }

    override func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
        guard let cell = collectionView.dequeueReusableCell(withReuseIdentifier: CNCellAndHeaderID.weekCalendarCell, for: indexPath) as? CNWeekViewCell else { return UICollectionViewCell() }
        cell.configureCell(withEvent: getCurrentEvent(with: indexPath) as? CNWeekViewObject)
        return cell
    }

    override func collectionView(_ collectionView: UICollectionView, viewForSupplementaryElementOfKind kind: String, at indexPath: IndexPath) -> UICollectionReusableView {
        let view: UICollectionReusableView
        switch kind {
        case CNWeekCalendarSupplementaryViewID.dateHeader:
            guard let columnHeader = collectionView.dequeueReusableSupplementaryView(ofKind: kind, withReuseIdentifier: kind, for: indexPath) as? CNDateHeader else { fatalError("CNALlDayHeader failed to be dequeued, WeekCalendarViewModel") }
            columnHeader.updateHeaderLabels(date: flowLayout.dateForColumnHeader(at: indexPath))
            view = columnHeader

        case JZSupplementaryViewKinds.rowHeader:
            guard let rowHeader = collectionView.dequeueReusableSupplementaryView(ofKind: kind, withReuseIdentifier: kind, for: indexPath) as? JZRowHeader else { fatalError("CNALlDayHeader failed to be dequeued, WeekCalendarViewModel") }
            rowHeader.updateView(date: flowLayout.timeForRowHeader(at: indexPath))
            view = rowHeader

        case CNWeekCalendarSupplementaryViewID.monthHeader:
            guard let cornerHeader = collectionView.dequeueReusableSupplementaryView(ofKind: kind, withReuseIdentifier: kind, for: indexPath) as? CNMonthHeader else { fatalError("CNMonthHeader failed to be dequeued, WeekCalendarViewModel") }
            cornerHeader.configureHeader(date: flowLayout.timeForRowHeader(at: indexPath), numOfDays: numOfDays)
            view = cornerHeader

        case JZSupplementaryViewKinds.currentTimeline:
            if currentTimelineType == .page {
                guard let currentTimeline = collectionView.dequeueReusableSupplementaryView(ofKind: kind, withReuseIdentifier: kind, for: indexPath) as? JZCurrentTimelinePage else { fatalError("CNALlDayHeader failed to be dequeued, WeekCalendarViewModel") }
                view = getPageTypeCurrentTimeline(timeline: currentTimeline, indexPath: indexPath)
            } else {
                guard let currentTimeline = collectionView.dequeueReusableSupplementaryView(ofKind: kind, withReuseIdentifier: kind, for: indexPath) as? JZCurrentTimelineSection else { fatalError("CNALlDayHeader failed to be dequeued, WeekCalendarViewModel") }
                view = getSectionTypeCurrentTimeline(timeline: currentTimeline, indexPath: indexPath)
            }
        case CNWeekCalendarSupplementaryViewID.allDayHeader:
            guard let alldayHeader = collectionView.dequeueReusableSupplementaryView(ofKind: kind, withReuseIdentifier: kind, for: indexPath) as? CNAllDayHeader else { fatalError("CNALlDayHeader failed to be dequeued, WeekCalendarViewModel") }
            let date = flowLayout.dateForColumnHeader(at: indexPath)
            guard let allDayEvents = allDayEventsBySection[date] as? [CNWeekViewObject] else {
                alldayHeader.update(withViews: [])
                return alldayHeader
            }
            alldayHeader.update(withViews: getAllDayHeaderViews(allDayEvents: allDayEvents))
            view = alldayHeader

        default:
            view = UICollectionReusableView()
        }
        return view
    }

    private func getAllDayHeaderViews(allDayEvents: [CNWeekViewObject]) -> [UIView] {
        var allDayViews = [UIView]()
        for event in allDayEvents {
            let view = CNWeekViewCell(frame: .zero)
            view.configureCell(withEvent: event, isAllDay: event.isAllDay)
            allDayViews.append(view)
        }
        return allDayViews
    }
}

extension WeekCalendarViewModel {
    // simple logic i implemented to allow the week view to bounce in the bottom, if bounce and alwaysBounceVertical is true.
    override func scrollViewDidScroll(_ scrollView: UIScrollView) {  
        if scrollView.contentOffset.y < 0 {
            scrollView.setContentOffset(CGPoint(x: scrollView.contentOffset.x, y: 0), animated: false)
        }
    }
}

Setup in view controller: where weekCalendar : WeekCalendarViewModel called in viewDidLoad() and I perform a forceReload() with new events in viewWillAppear().

    weekCalendar.translatesAutoresizingMaskIntoConstraints = false
    view.addSubview(weekCalendar)
    weekCalendar.topAnchor.constraint(equalTo: view.safeAreaLayoutGuide.topAnchor).isActive = true
    weekCalendar.bottomAnchor.constraint(equalTo: view.bottomAnchor).isActive = true
    weekCalendarLeading = weekCalendar.leadingAnchor.constraint(equalTo: view.leadingAnchor)
    weekCalendarTrailing = weekCalendar.trailingAnchor.constraint(equalTo: view.trailingAnchor)
    weekCalendarLeading.isActive = true
    weekCalendarTrailing.isActive = true
    
    weekCalendar.baseDelegate = self
    weekCalendar.setupCalendar(numOfDays: 7, setDate: Date(), allEvents: calendarObjectData ?? [Date : [CNWeekViewObject]](), scrollType: .pageScroll, firstDayOfWeek: .Monday, currentTimelineType: .page, visibleTime: Date())
    weekCalendar.updateFlowLayout(JZWeekViewFlowLayout(hourGridDivision: JZHourGridDivision.noneDiv))

initDidChange():

func initDateDidChange(_ weekView: JZBaseWeekView, initDate: Date) {
        print("initDidChange is called for date: \(initDate)")
        loadEventsFromCoreDataContainer(with: initDate)
        DispatchQueue.main.async {
            self.weekCalendar.forceReload(reloadEvents: self.calendarData)
        }
 }

Am I alone with this issue and need to debug further? Or did something change in the library that might have introduced this bug? Have I provided enough information?

Thanks for your time!

@zjfjack
Copy link
Owner

zjfjack commented Dec 14, 2018

Hi,

Actually this fix is quite important to keep the width of each grid the same.

I will look through this issue in the following weekend, and let you know soon.

Thanks

@theoadahl
Copy link
Author

Wonderful, thank you.

@zjfjack
Copy link
Owner

zjfjack commented Dec 16, 2018

Hello.

I've looked through your problem, but I am sorry that I cannot find any solutions yet.
However, I will give you some suggestions.

  1. I told you wrong about this 16cf27b update in the last message. Actually this update fix two issues. Let's only discuss the code in JZAllDayHeader.swift 16cf27b.
  • The code change from 36 to 40 lines actually resolve the constraints issue which will print out some conflict constraints warning in Xcode console because I set the height to 0 but the padding is actually 3. I do believe this will not affect your view.

  • For the change 56-62 lines, I think it might be the issue where it from. I changed this is just simply remove the scrollView when the AllDay events are 0, because some users don't use AllDay but those expensive scrollViews still need to be created, which might be more efficiency? You can simply discard this change to see whether it is the reason.

  1. For the initDate issue, I think it should not be the problem if you set the delegate(I saw it from your above code). It is obvious to see that I only put that delegate in initDate didSet method, it should be called, could you just simply add break point in JZBaseWeekView initDate to see whether the didSet has been called, then you might find the solution.

  2. For the jumps, I am still confused about your case, because I am not a native English speaker, you know, it quite hard to understand, haha. But, there is a quite important bug fix for all day height issue eb789aa, you can try the 9d303bd, because the previous one I made a mistake putting one print method in release code. If this update still not fix that issue, maybe can you make a small gif and show it to me?

Hopefully these opinions can help you to solve your issues.
If you get any solutions or issues, you can ask me again.

Thanks,
Jeff

@theoadahl
Copy link
Author

theoadahl commented Jan 10, 2019

Hello again! Sorry for taking so long, I've had much to do besides programming.

Thanks very much for your notes, I have looked through them and here comes a little update. A few of the problems seems to have disappeared with the newer versions by the way. I've not a native English speaker myself so the explanations may be a bit weird, hehe.

However I have looked further into this and narrowed down the problem. The code that creates the issue lies in JZBaseWeekView.swift.

Current issue update (using the same code to setup as above):

  1. Calendar scrollType is set to pageScroll. (sectionScroll does not create this issue).
  2. Using files from 2019-01-10. The JZBaseWeekView update introduces this issue.
  3. I added a breakpoint as you said in the didSet of the initDate. And as I scroll backwards (see below) it becomes clear that the initDate is not set when scrolling to the left.
  • I have further dissected the the issue to this: 1. When I scroll one page backwards (scrolling the calendar back in time) the initDateDidChange() delegate method is not called. 2. When I scroll one more page backwards I noticed that the 3 views that make up the calendar are not updated. Because I have reached the first view of the 3. 3. If I then scroll another time backwards initDateDidChange() is called and updates the view I'm on, but the calendar doesn't scroll. 4. 1-3 repeats.

  • Scrolling to the right (scrolling the calendar forward in time) does not raise any issues and works perfectly. The initDates didSet gets called as expected here.

I'll continue to see if I can find out why the initDate isn't set properly, when scrolling left.

@zjfjack
Copy link
Owner

zjfjack commented Jan 10, 2019

Hi.

I tested this on the current example project and everything works fine, but probably I misunderstand your meaning.

However, I think before you go further, maybe you can try the example project and set a break point to the didSet, if everything works fine, then it might be your problem.

If you find the example project still exists this problem, which means I do it wrong way. Let me know.

Thanks

@theoadahl
Copy link
Author

Of course i'll try that, you are probably right. I strongly believe I might be doing something wrong here too. Since the bug would be noticed very quickly if it was from the library itself, the reason i posted was just that the update seems to have caused for me. I shall try out the example project and as well more closely examine the changes from my cached version of the library.

@theoadahl
Copy link
Author

Hello again!

I have narrowed down the issue. Or at least I've found what caused it for me. I went through all commits til I found the one without the issue and I after that I compared the code in JZBaseWeekView.swift and noticed that if I use the latest version of the library but then comment out the flowLayout.rowHeaderWidth property change everything works as expected. I have yet to find the culprit... I have tested to change back from my custom views one by one, but that isn't causing. Well I'm gonna investigate further. But I believe it must have something to do with my implementation.

To demonstrate the change:

  • I have commented the line that cause my issue.

      open override func layoutSubviews() {
      super.layoutSubviews()
      
           flowLayout.sectionWidth = getSectionWidth()
           initialContentOffset = collectionView.contentOffset
      }
    
      private func getSectionWidth() -> CGFloat {
          var sectionWidth = contentViewWidth / CGFloat(numOfDays)
          let remainder = sectionWidth.truncatingRemainder(dividingBy: 1)
          switch remainder {
          case 0...0.25:
              sectionWidth = sectionWidth.rounded(.down)
          case 0.25...0.75:
              sectionWidth = sectionWidth.rounded(.down) + 0.5
          default:
              sectionWidth = sectionWidth.rounded(.up)
          }
          // Maximum added width for row header should be 0.25 * numberOfRows
          let rowHeaderWidth = frame.width - flowLayout.contentsMargin.left - flowLayout.contentsMargin.right - sectionWidth * CGFloat(numOfDays)
          
          // This is the line that causes the issue, if it is commented out everything works fine.
          flowLayout.rowHeaderWidth = rowHeaderWidth 
    
          return sectionWidth
      }
    

Thanks for your help and interest in the issue!

@zjfjack
Copy link
Owner

zjfjack commented Jan 13, 2019

Are you changing the rowHeaderWidth in another place?
For this code, it is used to fix #30 issue. For the reason, you can check this.

I think you might change this width in somewhere else, then it causes your issues.
For now, I can only explain the reason to you, and I don't know the main reason yet.
If you find out more, let me know. Thanks

@zjfjack
Copy link
Owner

zjfjack commented May 26, 2019

Hi mate,

I think the latest v0.7.0 should resolve your issue. If you are still interested, could you have a try and let me know? I completely redesign the pagination effect in this release.

You find the release comments here

@zjfjack zjfjack closed this as completed May 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants