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

automaticallyAdjustsContentOffset does not work anymore #23

Open
Dandiccf opened this issue Apr 18, 2017 · 10 comments
Open

automaticallyAdjustsContentOffset does not work anymore #23

Dandiccf opened this issue Apr 18, 2017 · 10 comments

Comments

@Dandiccf
Copy link

I have to insert multiple rows in my ASTableNode on index position 0 without animation and maintaining the users' visible scroll position.

In the latest AsyncDisplayKit (2.2.1) it is working perfectly when using

  • tableNode.view.automaticallyAdjustsContentOffset = true
  • tableNode.insertRows(at: prependIndexPaths, with: .none)

In Texture (2.3) I have following bugs

  • scroll position is jumping when inserting at index position 0 although using tableNode.view.automaticallyAdjustsContentOffset = true
  • the inserted rows have a transition animation although using UITableViewRowAnimation.none
@Dandiccf
Copy link
Author

it looks like there is no progress in this concern. Do you have it on your radar, oder is there an easy workaround? thx

@Dandiccf
Copy link
Author

Dandiccf commented Jun 9, 2017

am I the only one who uses 'automaticallyAdjustsContentOffset'?

It is still not working in 2.3.3.

Any comments on that, or am I doing something completely wrong?

thx

@nixzhu
Copy link
Contributor

nixzhu commented Dec 1, 2017

Same here, automaticallyAdjustsContentOffset is not working anymore.

@cesteban
Copy link
Contributor

I believe it stopped working with #317. By the time the content offset adjustment is applied, the visible nodes are still not updated (tableView:willDisplayCell:forRowAtIndexPath: won't be called until next layout pass), so the calculation of the offset of the first visible node in endAdjustingContentOffsetAnimated: just yields the same result than in beginAdjustingContentOffset, and nothing is done. Forcing a layout pass before endAdjusting... fixes the problem. We have been using this trick combined with manual contentOffset adjusting for weeks (we are in Texture 2.5). I would have a look later on what we have and consider sending a PR with this change.

I believe this is also (vaguely) related with the discussion about visibility on #788.

@aalenliang
Copy link

Any news on this issue?

@strangeliu
Copy link
Contributor

Any update?

@made2k
Copy link

made2k commented Feb 27, 2019

If someone finds this issue and is still seeing that deleting rows doesn't work with this, I've found a few things after this change set had been merged.

When deleting items from a ASTableNode, the internal _ASHierarchyChangeSet always sets its animated property to true. I noticed there's a TODO on that which may fix that issue.

https://github.com/TextureGroup/Texture/blob/master/Source/Private/_ASHierarchyChangeSet.h#L108

The key issue that causes is that Texture will not work like UIKit's UITableView when the changes are animated. It just gives up on adjusting the content inset.

https://github.com/TextureGroup/Texture/blob/master/Source/ASTableView.mm#L874

In my opinion, this issue is still valid since an ASTableNode still will not work with automaticallyAdjustsContentOffset (seemingly in any way at the moment).

For the animation always set to true issue, I noticed that by simply just calling

node.deleteRows(at: indexToDelete, with: .none) and seeing the change set's animation property set to YES.

@ay8s ay8s reopened this Feb 27, 2019
@ay8s
Copy link
Collaborator

ay8s commented Feb 27, 2019

Thanks @made2k. Was looking through some older issues and saw it had a merge associated.

@made2k
Copy link

made2k commented Feb 27, 2019

No problem, and thanks for opening it again!

I should also have noted in that first one if someone finds this, once that TODO gets handled in the _ASHierarchyChangeSet it looks like automaticallyAdjustsContentOffet will work, only when the changes aren't animated. That may be a viable workaround for some.

If that workaround doesn't work, there's still the ability to calculate your own adjusted offset based on the index paths that are being deleted (just grab the rect for row for each index that's above the user's current first visible index) and apply the sum of those heights to the current offset. However, with this approach the animation is a bit jumpy.

Hopefully that helps someone a little bit.

@JunyuKuang
Copy link

automaticallyAdjustsContentOffset works for me if I wrap the insertion/deletion code inside a performBatch block.

For example:

tableNode.performBatch(animated: false, updates: {
    items.insert(contentsOf: newItems, at: 0)
    let indexPaths = (0 ..< newItems.count).map { IndexPath(row: $0, section: 0) }
    tableNode.insertRows(at: indexPaths, with: .automatic)
}) { _ in }

I think the reason you have to do this is that automaticallyAdjustsContentOffset only applied to non-animated updates (see ASTableNode.h), while update with UITableView.RowAnimation.none will actually produce an animation (https://developer.apple.com/documentation/uikit/uitableview/rowanimation/none). Once performBatch disables the animation, automaticallyAdjustsContentOffset starts to work.

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

8 participants