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

heightForHeaderInSection call #71

Closed
makleso6 opened this issue Mar 2, 2020 · 7 comments
Closed

heightForHeaderInSection call #71

makleso6 opened this issue Mar 2, 2020 · 7 comments
Assignees

Comments

@makleso6
Copy link

makleso6 commented Mar 2, 2020

Hi!
Is it a bug that open func tableView(_ tableView: UITableView, heightForHeaderInSection section: Int) -> CGFloat called?

in simple implementation of UITableView with dataSource/delegate this method not call

@DenTelezhkin
Copy link
Owner

DenTelezhkin commented Mar 3, 2020

Hi!

This was actually intentional, but now as I'm thinking about it, I almost want to reconsider this decision. Some context:

Generally all delegate methods are not called by UITableView on DTTableViewManager, because the idea is to only call those methods, if user specified an action to go along with them. tableView(_ tableView: UITableView, heightForRowAt indexPath: IndexPath) is a glaring example of that, because as a framework owner I want to give access to modifying size of the cell via closure, but it should not be called for users that don't specify it, cause it might have huge performance implications.

However, in case of heightForHeaderInSection section and heightForFooterInSection section I made an exception from that rule. Currently those methods have a lot of logic in them. It includes:

  • Ability to not show header view/title on a section, whose header model is nil
  • Calling user defined closure for heightForHeaderInSection
  • Calling UITableViewDelegate method if it was defined by DTTableViewManageable instance
  • Logic to hide header view for grouped/plain table view style (minimalHeaderHeightForTableView property)
  • Returning UITableView.automaticDimension for default string table view header titles
  • Returning tableView.sectionHeaderHeight where header model is not nil.

If I made this method not callable from UITableView unless user specifies closure will make all this logic to go away. It's not a bad thing, maybe, because all of those can be achieved in different ways, like using self-sizing on tableView headers e.t.c. But this would be a breaking change, which I'm not looking to make at this particular moment.

As an option, I'm also considering putting this behind configuration flag, so that user of the framework can decide if he wants this behavior or not. Lets keep this issue open for now and think about it more. Please let me know what you think.

@makleso6
Copy link
Author

makleso6 commented Mar 3, 2020

In the first approximation, making a configuration flag is a good idea. I know that UITableView is black box with bugs running on differents iOS.

Can you explain why method call (I reproduce it in simple project by setting sectionHeaderHeight value), and how to fix it?

@DenTelezhkin
Copy link
Owner

Can you describe what behavior specifically are you trying to fix?

@makleso6
Copy link
Author

makleso6 commented Mar 3, 2020

I'll try to fix heightForHeaderInSection
In simple implantation of view controller with tableview first section of table view has default size ~35.0 and heightForHeaderInSection not call
But when I use DTTableViewManager heightForHeaderInSection call and returns .zero

@DenTelezhkin
Copy link
Owner

You need to provide a data model for header by using one of those methods (for MemoryStorage): memoryStorage.setSectionHeaderModels or memoryStorage.headerModelProvider property to make sure that it's visible.
You can also call manager.heightForHeader(withItem:_:) method to set height explicitly for each data model.

@DenTelezhkin
Copy link
Owner

Also you can check if you have any items in current section. If the section is empty, headers are not displayed by default, which is controlled via configuration flag manager.configuration.displayHeaderOnEmptySection.

@DenTelezhkin
Copy link
Owner

Hi!

I've decided to move forward with configuration flags for those methods, and planning to ship them in the next major release of DTTableViewManager(release later this fall, after iOS 14 / Xcode 12 GM).

Please take a look at current implementation. If there's anything you would want me to change there, please let me know.

I'm closing this issue for now, thanks for bringing this up!

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