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

More changes! (features + performance + general improvements) #127

Merged
merged 21 commits into from
Apr 4, 2012
Merged

Conversation

zadr
Copy link
Contributor

@zadr zadr commented Apr 4, 2012

This change set is rather big, and I'll be happy to break it up into a few smaller pull requests if desired. I think I managed to avoid using any tabs, so, the formatting should be good. And, of course, everything works fine in my testing, however, no one's perfect, and I may have missed things.

(Specifically: I do not have multiple selection enabled, and do my own custom highlighting/selection states in KKGridViewCell subclasses, so bugs may lie there.).

Anyway, please do let me know if there's anything else that needs changing with anything in this pull request, and without further ado, what was actually changed:

New

KKGridView

  • Add ability for rows to have backgrounds that take up the width of the gridview
Misc changes:

KKGridViewSectionInfo

  • Use @compatibility_alias instead of a set of empty subclasses for KKGridView{Footer, Header}

KKGridView

  • How cell selection/highlight takes place, to avoid selection on the Press -> Scroll -> Release flow
  • Avoid drawing a cell on half-pixel points
  • Use KVO instead of UIScrollViewDelegate methods
  • Only layout cells when the cell size/padding is set and changed, not every time it is set
Performance-related changes:

KKGridView

  • Cache -indexPathForIndex:0 inSection:0, since this is used a lot
  • Don't remove cells from the gridview; its faster to set them hidden and alpha to 0.

KKGridViewCell

  • Don't make _selectedBackgroundView or _contentView until it is first accessed by its getter (speeds up init)
  • Don't remove cells from the gridview; its faster to set them hidden and alpha to 0.

KKGridViewUpdateStack

  • Don't sort the update stack every time an item is added (allows for faster batching in addUpdates:)
  • Use a dictionary lookup for most operations, instead of iterating through an array over and over

Zach Drayer and others added 21 commits February 8, 2012 14:19
…identally press on something goes down 500x
…sed for row backgrounds while making sense as a name
The docs for [UITableViewDelegate tableView:willSelectRowAtIndexPath:]
say that you may “return nil if you don’t want the row selected”. That
makes sense, and given that KKGridView mimics the UITableView interface,
it’s nice to have the option to cancel the selection by returning
nil, too.
The docs for [UITableViewDelegate tableView:willSelectRowAtIndexPath:]
say that the “method is not called until users touch a row and then lift
their finger”. This makes sense, otherwise the callback is triggered
twice for each selection (once for touch down, once for touch up).
…ntil the property is either accessed, or the cell is pressed. Don't add selected background as a subview until pressed at all.
@jonsterling jonsterling merged commit 9756e28 into kolinkrewinkel:master Apr 4, 2012
@jonsterling
Copy link
Collaborator

Thanks for all your hard work!

@zadr
Copy link
Contributor Author

zadr commented Apr 4, 2012

Not a problem; thank you for making this in the first place!

@jonsterling
Copy link
Collaborator

There's a new issue with deletion… In the demo, try pressing the delete button, and then trying to scroll; all the cells in the first section start spinning around and freaking out.

I may have messed this up in some of my in-between changes, but I just wanted to see if you had any insight into this before I went looking.

@zadr
Copy link
Contributor Author

zadr commented Apr 4, 2012

Unsure. It works fine if the section goes offscreen, and comes back on screen. I can look into it a bit.

Also, unrelated, but, -_respondToBoundsChange calls -reloadData, and -setNeedsLayout, and -reloadData also calls -setNeedsLayout.

@jonsterling
Copy link
Collaborator

Thanks. I think @kolinkrewinkel is now looking into the issue I mentioned (#129).

As for the multiple calls to -setNeedsLayout, can you make a new issue for that? Then one of us can look into rejiggering that.

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

Successfully merging this pull request may close these issues.

3 participants