Skip to content

Commit

Permalink
[RTL/Batching] Make ASDisplayShouldFetchBatchForScrollView aware of f…
Browse files Browse the repository at this point in the history
…lipped CV layouts (TextureGroup#1985)

* [RTL/Batching] Make ASDisplayShouldFetchBatchForScrollView aware of flipped CV layouts

UICollectionViewLayout has a property called `flipsHorizontallyInOppositeLayoutDirection`. If this is set to `YES` then a RTL collectionView’s contentOffset behaves like it does in LTR. In other words, the first item is at contentOffset 0. In this case, the existing logic for `ASDisplayShouldFetchBatchForScrollView` works in RTL.

If you don’t override `flipsHorizontallyInOppositeLayoutDirection` to be `YES`, then it means that in RTL languages the first item in your collectionView will actually be at x offset `collectionView.contentSize.width - collectionView.frame.size.width`. As you scroll to the right, the content offset will decrease until you reach the end of the data at a content offset of 0,0. In this case, `ASDisplayShouldFetchBatchForScrollView` needs to know that you are in RTL and the layout is not flipped. It can then use the contentOffset as the `remainingDistance` to determine when to fetch.

* fix indentation

* assert that we are on main when accessing CV layout
  • Loading branch information
rcancro authored and OhKanghoon committed Mar 22, 2024
1 parent a59dacd commit 9cbc55d
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 38 deletions.
5 changes: 4 additions & 1 deletion Source/ASCollectionView.mm
Expand Up @@ -1857,7 +1857,10 @@ - (void)_checkForBatchFetching

- (void)_beginBatchFetchingIfNeededWithContentOffset:(CGPoint)contentOffset velocity:(CGPoint)velocity
{
if (ASDisplayShouldFetchBatchForScrollView(self, self.scrollDirection, self.scrollableDirections, contentOffset, velocity)) {
// Since we are accessing self.collectionViewLayout, we should make sure we are on main
ASDisplayNodeAssertMainThread();

if (ASDisplayShouldFetchBatchForScrollView(self, self.scrollDirection, self.scrollableDirections, contentOffset, velocity, self.collectionViewLayout.flipsHorizontallyInOppositeLayoutDirection)) {
[self _beginBatchFetching];
}
}
Expand Down
2 changes: 1 addition & 1 deletion Source/ASTableView.mm
Expand Up @@ -1485,7 +1485,7 @@ - (void)_checkForBatchFetching

- (void)_beginBatchFetchingIfNeededWithContentOffset:(CGPoint)contentOffset velocity:(CGPoint)velocity
{
if (ASDisplayShouldFetchBatchForScrollView(self, self.scrollDirection, ASScrollDirectionVerticalDirections, contentOffset, velocity)) {
if (ASDisplayShouldFetchBatchForScrollView(self, self.scrollDirection, ASScrollDirectionVerticalDirections, contentOffset, velocity, NO)) {
[self _beginBatchFetching];
}
}
Expand Down
8 changes: 7 additions & 1 deletion Source/Private/ASBatchFetching.h
Expand Up @@ -34,13 +34,16 @@ NS_ASSUME_NONNULL_BEGIN
@param scrollableDirections The possible scrolling directions of the scroll view.
@param contentOffset The offset that the scrollview will scroll to.
@param velocity The velocity of the scroll view (in points) at the moment the touch was released.
@param flipsHorizontallyInOppositeLayoutDirection Whether or not this scroll view flips its layout automatically in RTL.
See flipsHorizontallyInOppositeLayoutDirection in UICollectionViewLayout
@return Whether or not the current state should proceed with batch fetching.
*/
ASDK_EXTERN BOOL ASDisplayShouldFetchBatchForScrollView(UIScrollView<ASBatchFetchingScrollView> *scrollView,
ASScrollDirection scrollDirection,
ASScrollDirection scrollableDirections,
CGPoint contentOffset,
CGPoint velocity);
CGPoint velocity,
BOOL flipsHorizontallyInOppositeLayoutDirection);


/**
Expand All @@ -55,6 +58,8 @@ ASDK_EXTERN BOOL ASDisplayShouldFetchBatchForScrollView(UIScrollView<ASBatchFetc
@param visible Whether the view is visible or not.
@param velocity The velocity of the scroll view (in points) at the moment the touch was released.
@param delegate The delegate to be consulted if needed.
@param flipsHorizontallyInOppositeLayoutDirection Whether or not this scroll view flips its layout automatically in RTL.
See flipsHorizontallyInOppositeLayoutDirection in UICollectionViewLayout
@return Whether or not the current state should proceed with batch fetching.
@discussion This method is broken into a category for unit testing purposes and should be used with the ASTableView and
* ASCollectionView batch fetching API.
Expand All @@ -69,6 +74,7 @@ ASDK_EXTERN BOOL ASDisplayShouldFetchBatchForContext(ASBatchContext *context,
BOOL visible,
BOOL shouldRenderRTLLayout,
CGPoint velocity,
BOOL flipsHorizontallyInOppositeLayoutDirection,
_Nullable id<ASBatchFetchingDelegate> delegate);

NS_ASSUME_NONNULL_END
14 changes: 8 additions & 6 deletions Source/Private/ASBatchFetching.mm
Expand Up @@ -15,7 +15,8 @@ BOOL ASDisplayShouldFetchBatchForScrollView(UIScrollView<ASBatchFetchingScrollVi
ASScrollDirection scrollDirection,
ASScrollDirection scrollableDirections,
CGPoint contentOffset,
CGPoint velocity)
CGPoint velocity,
BOOL flipsHorizontallyInOppositeLayoutDirection)
{
// Don't fetch if the scroll view does not allow
if (![scrollView canBatchFetch]) {
Expand All @@ -30,7 +31,7 @@ BOOL ASDisplayShouldFetchBatchForScrollView(UIScrollView<ASBatchFetchingScrollVi
id<ASBatchFetchingDelegate> delegate = scrollView.batchFetchingDelegate;
BOOL visible = (scrollView.window != nil);
BOOL shouldRenderRTLLayout = [UIView userInterfaceLayoutDirectionForSemanticContentAttribute:scrollView.semanticContentAttribute] == UIUserInterfaceLayoutDirectionRightToLeft;
return ASDisplayShouldFetchBatchForContext(context, scrollDirection, scrollableDirections, bounds, contentSize, contentOffset, leadingScreens, visible, shouldRenderRTLLayout, velocity, delegate);
return ASDisplayShouldFetchBatchForContext(context, scrollDirection, scrollableDirections, bounds, contentSize, contentOffset, leadingScreens, visible, shouldRenderRTLLayout, velocity, flipsHorizontallyInOppositeLayoutDirection, delegate);
}

BOOL ASDisplayShouldFetchBatchForContext(ASBatchContext *context,
Expand All @@ -43,6 +44,7 @@ BOOL ASDisplayShouldFetchBatchForContext(ASBatchContext *context,
BOOL visible,
BOOL shouldRenderRTLLayout,
CGPoint velocity,
BOOL flipsHorizontallyInOppositeLayoutDirection,
id<ASBatchFetchingDelegate> delegate)
{
// Do not allow fetching if a batch is already in-flight and hasn't been completed or cancelled
Expand Down Expand Up @@ -87,11 +89,11 @@ BOOL ASDisplayShouldFetchBatchForContext(ASBatchContext *context,
}

CGFloat triggerDistance = viewLength * leadingScreens;
CGFloat remainingDistance = contentLength - viewLength - offset;
if (shouldRenderRTLLayout && ASScrollDirectionContainsHorizontalDirection(scrollableDirections)) {
remainingDistance = offset;
CGFloat remainingDistance = 0;
if (!flipsHorizontallyInOppositeLayoutDirection && shouldRenderRTLLayout && ASScrollDirectionContainsHorizontalDirection(scrollableDirections)) {
remainingDistance = offset;
} else {
remainingDistance = contentLength - viewLength - offset;
remainingDistance = contentLength - viewLength - offset;
}
BOOL result = remainingDistance <= triggerDistance;

Expand Down

0 comments on commit 9cbc55d

Please sign in to comment.