Skip to content

Commit

Permalink
Merge pull request #36534 from margelo/@kirillzyusko/simplify-native-…
Browse files Browse the repository at this point in the history
…attachment-carousel

Simplify native attachment gallery paging context and improve code
  • Loading branch information
pecanoro authored Feb 15, 2024
2 parents 954b1dd + 1ec5883 commit fe1cb5b
Show file tree
Hide file tree
Showing 15 changed files with 213 additions and 206 deletions.
15 changes: 1 addition & 14 deletions src/components/Attachments/AttachmentCarousel/CarouselItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,6 @@ const propTypes = {
transactionID: PropTypes.string,
}).isRequired,

/** Whether there is only one element in the attachment carousel */
isSingleItem: PropTypes.bool.isRequired,

/** The index of the carousel item */
index: PropTypes.number.isRequired,

/** The index of the currently active carousel item */
activeIndex: PropTypes.number.isRequired,

/** onPress callback */
onPress: PropTypes.func,
};
Expand All @@ -54,7 +45,7 @@ const defaultProps = {
onPress: undefined,
};

function CarouselItem({item, index, activeIndex, isSingleItem, onPress}) {
function CarouselItem({item, onPress}) {
const styles = useThemeStyles();
const {translate} = useLocalize();
const {isAttachmentHidden} = useContext(ReportAttachmentsContext);
Expand Down Expand Up @@ -104,10 +95,6 @@ function CarouselItem({item, index, activeIndex, isSingleItem, onPress}) {
source={item.source}
file={item.file}
isAuthTokenRequired={item.isAuthTokenRequired}
isUsedInCarousel
isSingleCarouselItem={isSingleItem}
carouselItemIndex={index}
carouselActiveItemIndex={activeIndex}
onPress={onPress}
transactionID={item.transactionID}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,24 @@ import {createContext} from 'react';
import type PagerView from 'react-native-pager-view';
import type {SharedValue} from 'react-native-reanimated';

/** The pager items array is used within the pager to render and navigate between the images */
type AttachmentCarouselPagerItems = {
/** The source of the image is used to identify each attachment/page in the pager */
source: string;

/** The index of the pager item determines the order of the images in the pager */
index: number;

/** The active state of the pager item determines whether the image is currently transformable with pinch, pan and tap gestures */
isActive: boolean;
};

type AttachmentCarouselPagerContextValue = {
/** The list of items that are shown in the pager */
pagerItems: AttachmentCarouselPagerItems[];

/** The index of the active page */
activePage: number;
pagerRef: ForwardedRef<PagerView>;
isPagerScrolling: SharedValue<boolean>;
isScrollEnabled: SharedValue<boolean>;
Expand Down
72 changes: 48 additions & 24 deletions src/components/Attachments/AttachmentCarousel/Pager/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {createNativeWrapper} from 'react-native-gesture-handler';
import type {PagerViewProps} from 'react-native-pager-view';
import PagerView from 'react-native-pager-view';
import Animated, {useAnimatedProps, useSharedValue} from 'react-native-reanimated';
import CarouselItem from '@components/Attachments/AttachmentCarousel/CarouselItem';
import useThemeStyles from '@hooks/useThemeStyles';
import AttachmentCarouselPagerContext from './AttachmentCarouselPagerContext';
import usePageScrollHandler from './usePageScrollHandler';
Expand All @@ -19,30 +20,40 @@ type AttachmentCarouselPagerHandle = {
setPage: (selectedPage: number) => void;
};

type PagerItem = {
key: string;
url: string;
type Attachment = {
source: string;
};

type AttachmentCarouselPagerProps = {
items: PagerItem[];
renderItem: (props: {item: PagerItem; index: number; isActive: boolean}) => React.ReactNode;
initialIndex: number;
/** The attachments to be rendered in the pager. */
items: Attachment[];

/** The source (URL) of the currently active attachment. */
activeSource: string;

/** The index of the initial page to be rendered. */
initialPage: number;

/** A callback to be called when the page is changed. */
onPageSelected: () => void;

/**
* A callback that can be used to toggle the attachment carousel arrows, when the scale of the image changes.
* @param showArrows If set, it will show/hide the arrows. If not set, it will toggle the arrows.
*/
onRequestToggleArrows: (showArrows?: boolean) => void;
};

function AttachmentCarouselPager({items, renderItem, initialIndex, onPageSelected, onRequestToggleArrows}: AttachmentCarouselPagerProps, ref: ForwardedRef<AttachmentCarouselPagerHandle>) {
function AttachmentCarouselPager({items, activeSource, initialPage, onPageSelected, onRequestToggleArrows}: AttachmentCarouselPagerProps, ref: ForwardedRef<AttachmentCarouselPagerHandle>) {
const styles = useThemeStyles();
const pagerRef = useRef<PagerView>(null);

const scale = useRef(1);
const isPagerScrolling = useSharedValue(false);
const isScrollEnabled = useSharedValue(true);

const activePage = useSharedValue(initialIndex);
const [activePageState, setActivePageState] = useState(initialIndex);
const activePage = useSharedValue(initialPage);
const [activePageIndex, setActivePageIndex] = useState(initialPage);

const pageScrollHandler = usePageScrollHandler((e) => {
'worklet';
Expand All @@ -52,9 +63,12 @@ function AttachmentCarouselPager({items, renderItem, initialIndex, onPageSelecte
}, []);

useEffect(() => {
setActivePageState(initialIndex);
activePage.value = initialIndex;
}, [activePage, initialIndex]);
setActivePageIndex(initialPage);
activePage.value = initialPage;
}, [activePage, initialPage]);

/** The `pagerItems` object that passed down to the context. Later used to detect current page, whether it's a single image gallery etc. */
const pagerItems = useMemo(() => items.map((item, index) => ({source: item.source, index, isActive: index === activePageIndex})), [activePageIndex, items]);

/**
* This callback is passed to the MultiGestureCanvas/Lightbox through the AttachmentCarouselPagerContext.
Expand Down Expand Up @@ -94,13 +108,15 @@ function AttachmentCarouselPager({items, renderItem, initialIndex, onPageSelecte

const contextValue = useMemo(
() => ({
pagerRef,
pagerItems,
activePage: activePageIndex,
isPagerScrolling,
isScrollEnabled,
pagerRef,
onTap: handleTap,
onScaleChanged: handleScaleChange,
}),
[isPagerScrolling, isScrollEnabled, handleTap, handleScaleChange],
[pagerItems, activePageIndex, isPagerScrolling, isScrollEnabled, handleTap, handleScaleChange],
);

const animatedProps = useAnimatedProps(() => ({
Expand All @@ -121,26 +137,34 @@ function AttachmentCarouselPager({items, renderItem, initialIndex, onPageSelecte
[],
);

const carouselItems = items.map((item, index) => (
<View
key={item.source}
style={styles.flex1}
>
<CarouselItem
// @ts-expect-error TODO: Remove this once AttachmentView (https://github.com/Expensify/App/issues/25150) is migrated to TypeScript.
item={item}
isSingleItem={items.length === 1}
index={index}
isFocused={index === activePageIndex && activeSource === item.source}
/>
</View>
));

return (
<AttachmentCarouselPagerContext.Provider value={contextValue}>
<AnimatedPagerView
pageMargin={40}
offscreenPageLimit={1}
onPageScroll={pageScrollHandler}
onPageSelected={onPageSelected}
ref={pagerRef}
style={styles.flex1}
initialPage={initialIndex}
initialPage={initialPage}
animatedProps={animatedProps}
ref={pagerRef}
>
{items.map((item, index) => (
<View
key={item.source}
style={styles.flex1}
>
{renderItem({item, index, isActive: index === activePageState})}
</View>
))}
{carouselItems}
</AnimatedPagerView>
</AttachmentCarouselPagerContext.Provider>
);
Expand Down
23 changes: 2 additions & 21 deletions src/components/Attachments/AttachmentCarousel/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import variables from '@styles/variables';
import ONYXKEYS from '@src/ONYXKEYS';
import {defaultProps, propTypes} from './attachmentCarouselPropTypes';
import CarouselButtons from './CarouselButtons';
import CarouselItem from './CarouselItem';
import extractAttachmentsFromReport from './extractAttachmentsFromReport';
import AttachmentCarouselPager from './Pager';
import useCarouselArrows from './useCarouselArrows';
Expand Down Expand Up @@ -103,24 +102,6 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source,
[setShouldShowArrows],
);

/**
* Defines how a single attachment should be rendered
* @param {{ reportActionID: String, isAuthTokenRequired: Boolean, source: String, file: { name: String }, hasBeenFlagged: Boolean }} item
* @returns {JSX.Element}
*/
const renderItem = useCallback(
({item, index, isActive}) => (
<CarouselItem
item={item}
isSingleItem={attachments.length === 1}
index={index}
activeIndex={page}
isFocused={isActive && activeSource === item.source}
/>
),
[activeSource, attachments.length, page],
);

return (
<View style={[styles.flex1, styles.attachmentCarouselContainer]}>
{page == null ? (
Expand Down Expand Up @@ -148,8 +129,8 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source,

<AttachmentCarouselPager
items={attachments}
renderItem={renderItem}
initialIndex={page}
initialPage={page}
activeSource={activeSource}
onRequestToggleArrows={toggleArrows}
onPageSelected={({nativeEvent: {position: newPage}}) => updatePage(newPage)}
ref={pagerRef}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,7 @@ const propTypes = {
...withLocalizePropTypes,
};

function AttachmentViewImage({
url,
file,
isAuthTokenRequired,
isUsedInCarousel,
isSingleCarouselItem,
carouselItemIndex,
carouselActiveItemIndex,
isFocused,
loadComplete,
onPress,
onError,
isImage,
translate,
}) {
function AttachmentViewImage({url, file, isAuthTokenRequired, isFocused, loadComplete, onPress, onError, isImage, translate}) {
const styles = useThemeStyles();
const children = (
<ImageView
Expand All @@ -35,10 +21,6 @@ function AttachmentViewImage({
fileName={file.name}
isAuthTokenRequired={isImage && isAuthTokenRequired}
isFocused={isFocused}
isUsedInCarousel={isUsedInCarousel}
isSingleCarouselItem={isSingleCarouselItem}
carouselItemIndex={carouselItemIndex}
carouselActiveItemIndex={carouselActiveItemIndex}
/>
);

Expand Down
8 changes: 0 additions & 8 deletions src/components/Attachments/AttachmentView/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,6 @@ function AttachmentView({
translate,
isFocused,
isUsedInCarousel,
isSingleCarouselItem,
carouselItemIndex,
carouselActiveItemIndex,
isUsedInAttachmentModal,
isWorkspaceAvatar,
maybeIcon,
Expand Down Expand Up @@ -147,8 +144,6 @@ function AttachmentView({
isFocused={isFocused}
isAuthTokenRequired={isAuthTokenRequired}
encryptedSourceUrl={encryptedSourceUrl}
carouselItemIndex={carouselItemIndex}
carouselActiveItemIndex={carouselActiveItemIndex}
onPress={onPress}
onToggleKeyboard={onToggleKeyboard}
onLoadComplete={() => !loadComplete && setLoadComplete(true)}
Expand Down Expand Up @@ -177,9 +172,6 @@ function AttachmentView({
loadComplete={loadComplete}
isFocused={isFocused}
isUsedInCarousel={isUsedInCarousel}
isSingleCarouselItem={isSingleCarouselItem}
carouselItemIndex={carouselItemIndex}
carouselActiveItemIndex={carouselActiveItemIndex}
isImage={isImage}
onPress={onPress}
onError={() => {
Expand Down
17 changes: 1 addition & 16 deletions src/components/Attachments/AttachmentView/propTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,6 @@ const attachmentViewPropTypes = {
/** Whether this AttachmentView is shown as part of a AttachmentCarousel */
isUsedInCarousel: PropTypes.bool,

/** When "isUsedInCarousel" is set to true, determines whether there is only one item in the carousel */
isSingleCarouselItem: PropTypes.bool,

/** Whether this AttachmentView is shown as part of an AttachmentModal */
isUsedInAttachmentModal: PropTypes.bool,

/** The index of the carousel item */
carouselItemIndex: PropTypes.number,

/** The index of the currently active carousel item */
carouselActiveItemIndex: PropTypes.number,

/** Function for handle on press */
onPress: PropTypes.func,

Expand All @@ -39,11 +27,8 @@ const attachmentViewDefaultProps = {
name: '',
},
isFocused: false,
isUsedInCarousel: false,
isSingleCarouselItem: false,
carouselItemIndex: 0,
carouselActiveItemIndex: 0,
isSingleElement: false,
isUsedInCarousel: false,
isUsedInAttachmentModal: false,
onPress: undefined,
onScaleChanged: () => {},
Expand Down
17 changes: 1 addition & 16 deletions src/components/ImageView/index.native.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,13 @@ import Lightbox from '@components/Lightbox';
import {DEFAULT_ZOOM_RANGE} from '@components/MultiGestureCanvas';
import type {ImageViewProps} from './types';

function ImageView({
isAuthTokenRequired = false,
url,
style,
zoomRange = DEFAULT_ZOOM_RANGE,
onError,
isUsedInCarousel = false,
isSingleCarouselItem = false,
carouselItemIndex = 0,
carouselActiveItemIndex = 0,
}: ImageViewProps) {
const hasSiblingCarouselItems = isUsedInCarousel && !isSingleCarouselItem;

function ImageView({isAuthTokenRequired = false, url, style, zoomRange = DEFAULT_ZOOM_RANGE, onError}: ImageViewProps) {
return (
<Lightbox
uri={url}
zoomRange={zoomRange}
isAuthTokenRequired={isAuthTokenRequired}
onError={onError}
index={carouselItemIndex}
activeIndex={carouselActiveItemIndex}
hasSiblingCarouselItems={hasSiblingCarouselItems}
style={style}
/>
);
Expand Down
12 changes: 0 additions & 12 deletions src/components/ImageView/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,6 @@ type ImageViewProps = {
/** Handles errors while displaying the image */
onError?: () => void;

/** Whether this AttachmentView is shown as part of a AttachmentCarousel */
isUsedInCarousel?: boolean;

/** When "isUsedInCarousel" is set to true, determines whether there is only one item in the carousel */
isSingleCarouselItem?: boolean;

/** The index of the carousel item */
carouselItemIndex?: number;

/** The index of the currently active carousel item */
carouselActiveItemIndex?: number;

/** Additional styles to add to the component */
style?: StyleProp<ViewStyle>;

Expand Down
Loading

0 comments on commit fe1cb5b

Please sign in to comment.