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

Paging items do not get resized properly after reloading data #429

Closed
rptoma opened this issue Feb 26, 2020 · 11 comments
Closed

Paging items do not get resized properly after reloading data #429

rptoma opened this issue Feb 26, 2020 · 11 comments

Comments

@rptoma
Copy link

rptoma commented Feb 26, 2020

Consider the following scenario:

  1. The paging view controller gets loaded with some data.
  2. The data changes (including the number of pages/paging items) .
  3. So, we call reloadData on the paging view controller.

Everything gets reloaded, the number of paging items get updated but they are not sized correctly. My guess is that the function that specifies widthForPagingItem does not get recalled after reload.

The only solution is to completely remove and re-add the view controller which is not ok at all.

Thank you.

@rechsteiner
Copy link
Owner

Thanks for the report. I'll look into getting this fixed 🙂

@rechsteiner
Copy link
Owner

Which version of Parchment are you using? I just tried reproducing this using one of the example targets, but it seems to work just fine. Here's the code I'm running (you can trigger a memory warning in the simulator to toggle the states):

class ViewController: UIViewController {
  var expanded: Bool = false
  let pagingViewController = PagingViewController(viewControllers: [
    IndexViewController(index: 0),
    IndexViewController(index: 1)
  ])
  
  override func viewDidLoad() {
    super.viewDidLoad()
    pagingViewController.sizeDelegate = self
    addChild(pagingViewController)
    view.addSubview(pagingViewController.view)
    view.constrainToEdges(pagingViewController.view)
    pagingViewController.didMove(toParent: self)
  }
  
  override func didReceiveMemoryWarning() {
    expanded.toggle()
    pagingViewController.reloadData()
  }
}

extension ViewController: PagingViewControllerSizeDelegate {
  func pagingViewController(_: PagingViewController, widthForPagingItem pagingItem: PagingItem, isSelected: Bool) -> CGFloat {
    return expanded ? 200 : 100
  }
}

Do you see anything I'm missing for reproducing this?

@rptoma
Copy link
Author

rptoma commented Mar 6, 2020

Hey,

Thanks for checking it out.

The currently used version is 2.0.1, but it happened before, too (on 1.7.0).

The differences between what we do and your example are that:

  1. We are using PagingViewControllerDataSource for setting the view controllers
  2. We are using custom cells for the menu items (they contain an image and a label)

Thanks!

@rechsteiner
Copy link
Owner

Tried setting up a data source and using custom cells, but it's still not able to reproduce this. Here's the current code I'm running:

class ViewController: UIViewController {
  var expanded: Bool = false
  let pagingViewController = PagingViewController()
  
  class Cell: PagingCell {
    override func setPagingItem(_ pagingItem: PagingItem, selected: Bool, options: PagingOptions) {
      contentView.backgroundColor = .green
    }
  }
  
  override func viewDidLoad() {
    super.viewDidLoad()
    pagingViewController.dataSource = self
    pagingViewController.sizeDelegate = self
    pagingViewController.register(Cell.self, for: PagingIndexItem.self)
    pagingViewController.menuItemSpacing = 5
    
    addChild(pagingViewController)
    view.addSubview(pagingViewController.view)
    view.constrainToEdges(pagingViewController.view)
    pagingViewController.didMove(toParent: self)
  }
  
  override func didReceiveMemoryWarning() {
    expanded.toggle()
    pagingViewController.reloadData()
  }
}

extension ViewController: PagingViewControllerDataSource {
  
  func pagingViewController(_: PagingViewController, pagingItemAt index: Int) -> PagingItem {
    return PagingIndexItem(index: index, title: "View \(index)")
  }
  
  func pagingViewController(_: PagingViewController, viewControllerAt index: Int) -> UIViewController {
    return IndexViewController(index: index)
  }
  
  func numberOfViewControllers(in pagingViewController: PagingViewController) -> Int {
    return 3
  }
  
}

extension ViewController: PagingViewControllerSizeDelegate {
  func pagingViewController(_: PagingViewController, widthForPagingItem pagingItem: PagingItem, isSelected: Bool) -> CGFloat {
    return expanded ? 200 : 100
  }
}

Any other ideas what might be different from your app?

@grifas
Copy link

grifas commented Mar 10, 2020

Same issue for me but apparently it's an other library causes the bug (pod SkeletonView).

@rptoma
Copy link
Author

rptoma commented Mar 10, 2020

@rechsteiner Not really. Something weird that I remember I noticed was that the first cells looked OK, but after scrolling for a little bit, the cells have wrong dimensions (i.e. too small to fit the content).

@grifas We don't use SkeletonView and it doesn't seem to be a dependency of Parchment. 🤔 Also, don't see how it might affect it because as far as I understand SkeletonView is a placeholder for the table view.

@grifas
Copy link

grifas commented Mar 10, 2020

@rptoma Yeah I know but a line of this library causes the issue for my project. I didn't dig more into that but I just commented the line and I saw no more issue. I just thought maybe you are in the same case as me.

@rechsteiner
Copy link
Owner

Hmm, okay. Could you share some examples of how you are calculating the width using the size delegate? Also, are you using a custom PagingItem as your data source?

@rechsteiner
Copy link
Owner

I've made some changes to the way we're invalidating the size cache now (#444). Could you try to target the master branch and see if this fixes the issue you're facing?

@rptoma
Copy link
Author

rptoma commented Mar 11, 2020

It is solved now! Awesome, thanks! Looking forward to the new release.

@rechsteiner
Copy link
Owner

Awesome! Thanks for helping me debug. The fix was just released in the latest version now: https://github.com/rechsteiner/Parchment/releases/tag/v2.1.0

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

3 participants