Skip to content

Fix/page carousel RTL android #1612

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

Merged
merged 5 commits into from
Oct 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ interface State {
asCarousel: boolean;
centerSelected: boolean;
fewItems: boolean;
initialIndex: number;
selectedIndex: number;
key: string | number;
items: TabControllerItemProps[];
Expand All @@ -26,6 +27,7 @@ class TabControllerScreen extends Component<{}, State> {
asCarousel: true,
centerSelected: false,
fewItems: false,
initialIndex: 0,
selectedIndex: 0,
key: Date.now(),
items: []
Expand Down Expand Up @@ -146,14 +148,14 @@ class TabControllerScreen extends Component<{}, State> {
}

render() {
const {key, /* selectedIndex, */ asCarousel, centerSelected, fewItems, items} = this.state;
const {key, initialIndex, /* selectedIndex, */ asCarousel, centerSelected, fewItems, items} = this.state;
return (
<View flex bg-grey70>
<TabController
key={key}
asCarousel={asCarousel}
// selectedIndex={selectedIndex}
initialIndex={0}
initialIndex={initialIndex}
onChangeIndex={this.onChangeIndex}
items={items}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@ interface TabControllerContext {
initialIndex?: number;
selectedIndex?: number;
items?: any[];
itemsCount: number;
asCarousel?: boolean;
containerWidth: number;
pageWidth: number;
/** static page index */
currentPage: Reanimated.SharedValue<number>;
/** transition page index (can be a fraction when transitioning between pages) */
targetPage: Reanimated.SharedValue<number>;
carouselOffset: Reanimated.SharedValue<number>;
setCurrentIndex: (index: number) => void;
}
declare const TabBarContext: React.Context<TabControllerContext>;
export default TabBarContext;
63 changes: 45 additions & 18 deletions src/components/tabController/PageCarousel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ import Reanimated, {
useAnimatedReaction,
useAnimatedRef,
useAnimatedScrollHandler,
useSharedValue,
withTiming
useSharedValue
} from 'react-native-reanimated';
import {Constants} from 'helpers';

const FIX_RTL = Constants.isRTL && Constants.isAndroid;

/**
* @description: TabController's Page Carousel
Expand All @@ -17,46 +19,70 @@ import Reanimated, {
function PageCarousel({...props}) {
const carousel = useAnimatedRef<Reanimated.ScrollView>();
const {
itemsCount,
currentPage,
targetPage,
selectedIndex = 0,
pageWidth,
carouselOffset
// carouselOffset,
setCurrentIndex
} = useContext(TabBarContext);
const contentOffset = useMemo(() => ({x: selectedIndex * pageWidth, y: 0}), [selectedIndex, pageWidth]);
const wasScrolledByPress = useSharedValue(false);
const initialOffset = useMemo(() => ({x: currentPage.value * pageWidth, y: 0}), []);
const indexChangeReason = useSharedValue<'byScroll' | 'byPress' | undefined>(undefined);

const scrollToInitial = useCallback(() => {
if (Constants.isAndroid && currentPage.value) {
scrollToItem(currentPage.value);
}
}, []);

const calcOffset = useCallback(offset => {
'worklet';
return FIX_RTL ? pageWidth * (itemsCount - 1) - offset : offset;
},
[pageWidth, itemsCount]);

const scrollHandler = useAnimatedScrollHandler({
onScroll: e => {
carouselOffset.value = e.contentOffset.x;
const newIndex = e.contentOffset.x / pageWidth;
// carouselOffset.value = e.contentOffset.x;
const xOffset = calcOffset(e.contentOffset.x);
const newIndex = xOffset / pageWidth;

if (wasScrolledByPress.value) {
if (indexChangeReason.value === 'byPress') {
// Scroll was immediate and not by gesture
/* Round is for android when offset value has fraction */
targetPage.value = withTiming(Math.round(newIndex));
wasScrolledByPress.value = false;
// targetPage.value = withTiming(Math.round(newIndex));

indexChangeReason.value = undefined;
} else {
targetPage.value = newIndex;
}
},
onMomentumEnd: e => {
const newPage = Math.round(e.contentOffset.x / pageWidth);
currentPage.value = newPage;
const xOffset = calcOffset(e.contentOffset.x);
const newPage = Math.round(xOffset / pageWidth);
indexChangeReason.value = 'byScroll';
setCurrentIndex(newPage);
}
});

const scrollToItem = useCallback(index => {
wasScrolledByPress.value = true;
if (indexChangeReason.value === 'byScroll') {
indexChangeReason.value = undefined;
} else {
indexChangeReason.value = 'byPress';
}

const actualIndex = FIX_RTL ? itemsCount - index - 1 : index;
// @ts-expect-error
carousel.current?.scrollTo({x: index * pageWidth, animated: false});
carousel.current?.scrollTo({x: actualIndex * pageWidth, animated: false});
},
[pageWidth]);
[pageWidth, itemsCount]);

useAnimatedReaction(() => {
return currentPage.value;
},
(currIndex, prevIndex) => {
if (currIndex !== prevIndex) {
if (prevIndex !== null && currIndex !== prevIndex) {
runOnJS(scrollToItem)(currIndex);
}
});
Expand All @@ -75,7 +101,8 @@ function PageCarousel({...props}) {
showsHorizontalScrollIndicator={false}
onScroll={scrollHandler}
scrollEventThrottle={16}
contentOffset={contentOffset} // iOS only
contentOffset={initialOffset} // iOS only
onLayout={scrollToInitial} // Android only
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Android with RTL has a visible change of page with initialIndex; according to the docs, contentOffset should work for Android, but it doesn't, should we create a ticket to check-up on it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a very old issue..
not sure when they will even support that.
I wouldn't try to hard to solve that.. at least for now

/>
);
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/tabController/TabBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ const TabBar = (props: Props) => {
return Math.round(currentPage.value);
},
(currIndex, prevIndex) => {
if (currIndex !== prevIndex) {
if (prevIndex !== null && currIndex !== prevIndex) {
runOnJS(focusIndex)(currIndex);
}
});
Expand Down
4 changes: 3 additions & 1 deletion src/components/tabController/TabBarContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,16 @@ interface TabControllerContext {
// DEPRECATED: use initialIndex instead
selectedIndex?: number;
items?: any[];
itemsCount: number;
asCarousel?: boolean;
containerWidth: number;
pageWidth: number;
/** static page index */
currentPage: Reanimated.SharedValue<number>;
/** transition page index (can be a fraction when transitioning between pages) */
targetPage: Reanimated.SharedValue<number>;
carouselOffset: Reanimated.SharedValue<number>;
/* carouselOffset: Reanimated.SharedValue<number>; */
setCurrentIndex: (index: number) => void;
}

// @ts-expect-error
Expand Down
4 changes: 2 additions & 2 deletions src/components/tabController/TabBarItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ export default function TabBarItem({
style,
...props
}: Props) {
const {currentPage} = useContext(TabBarContext);
const {currentPage, setCurrentIndex} = useContext(TabBarContext);
const itemRef = useRef();
const itemWidth = useRef(props.width);
// JSON.parse(JSON.stringify is due to an issue with reanimated
Expand All @@ -140,7 +140,7 @@ export default function TabBarItem({

const onPress = useCallback(() => {
if (!ignore) {
currentPage.value = index;
setCurrentIndex(index);
}

props.onPress?.(index);
Expand Down
17 changes: 12 additions & 5 deletions src/components/tabController/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// TODO: support commented props
import React, {PropsWithChildren, useMemo, useEffect, useRef, useState} from 'react';
import React, {PropsWithChildren, useMemo, useEffect, useRef, useState, useCallback} from 'react';
import _ from 'lodash';
import {useAnimatedReaction, useSharedValue, withTiming, runOnJS} from 'react-native-reanimated';
import {Constants} from '../../helpers';
Expand Down Expand Up @@ -93,7 +93,12 @@ function TabController({
const currentPage = useSharedValue(initialIndex);
/* targetPage - transitioned page index (can be a fraction when transitioning between pages) */
const targetPage = useSharedValue(initialIndex);
const carouselOffset = useSharedValue(initialIndex * Math.round(pageWidth));
// const carouselOffset = useSharedValue(initialIndex * Math.round(pageWidth));

const setCurrentIndex = useCallback(index => {
'worklet';
currentPage.value = index;
}, []);

useEffect(() => {
if (!_.isUndefined(selectedIndex)) {
Expand All @@ -102,7 +107,7 @@ function TabController({
}, [selectedIndex]);

useEffect(() => {
currentPage.value = initialIndex;
setCurrentIndex(initialIndex);
}, [initialIndex]);

useAnimatedReaction(() => {
Expand All @@ -124,13 +129,15 @@ function TabController({
/* Items */
items,
ignoredItems,
itemsCount: items.length - ignoredItems.length,
/* Animated Values */
targetPage,
currentPage,
carouselOffset,
// carouselOffset,
containerWidth: screenWidth,
/* Callbacks */
onChangeIndex
onChangeIndex,
setCurrentIndex
};
}, [initialIndex, asCarousel, items, onChangeIndex, screenWidth]);

Expand Down
2 changes: 1 addition & 1 deletion src/components/tabController/useScrollToItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ const useScrollToItem = <T extends ScrollToSupportedViews>(props: ScrollToItemPr

useEffect(() => {
if (!_.isUndefined(selectedIndex)) {
focusIndex(selectedIndex);
focusIndex(selectedIndex, false);
}
}, [selectedIndex, focusIndex]);

Expand Down