Skip to content

Commit

Permalink
Revert "Simplify native attachment gallery paging context and improve…
Browse files Browse the repository at this point in the history
… code"
  • Loading branch information
tgolen authored Feb 14, 2024
1 parent 8277ba5 commit e609383
Show file tree
Hide file tree
Showing 15 changed files with 206 additions and 213 deletions.
15 changes: 14 additions & 1 deletion src/components/Attachments/AttachmentCarousel/CarouselItem.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,15 @@ 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 @@ -45,7 +54,7 @@ const defaultProps = {
onPress: undefined,
};

function CarouselItem({item, onPress}) {
function CarouselItem({item, index, activeIndex, isSingleItem, onPress}) {
const styles = useThemeStyles();
const {translate} = useLocalize();
const {isAttachmentHidden} = useContext(ReportAttachmentsContext);
Expand Down Expand Up @@ -95,6 +104,10 @@ function CarouselItem({item, 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,24 +3,7 @@ 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: 24 additions & 48 deletions src/components/Attachments/AttachmentCarousel/Pager/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ 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 @@ -20,40 +19,30 @@ type AttachmentCarouselPagerHandle = {
setPage: (selectedPage: number) => void;
};

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

type AttachmentCarouselPagerProps = {
/** 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. */
items: PagerItem[];
renderItem: (props: {item: PagerItem; index: number; isActive: boolean}) => React.ReactNode;
initialIndex: number;
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, activeSource, initialPage, onPageSelected, onRequestToggleArrows}: AttachmentCarouselPagerProps, ref: ForwardedRef<AttachmentCarouselPagerHandle>) {
function AttachmentCarouselPager({items, renderItem, initialIndex, 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(initialPage);
const [activePageIndex, setActivePageIndex] = useState(initialPage);
const activePage = useSharedValue(initialIndex);
const [activePageState, setActivePageState] = useState(initialIndex);

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

useEffect(() => {
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]);
setActivePageState(initialIndex);
activePage.value = initialIndex;
}, [activePage, initialIndex]);

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

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

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

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={initialPage}
initialPage={initialIndex}
animatedProps={animatedProps}
ref={pagerRef}
>
{carouselItems}
{items.map((item, index) => (
<View
key={item.source}
style={styles.flex1}
>
{renderItem({item, index, isActive: index === activePageState})}
</View>
))}
</AnimatedPagerView>
</AttachmentCarouselPagerContext.Provider>
);
Expand Down
23 changes: 21 additions & 2 deletions src/components/Attachments/AttachmentCarousel/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ 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 @@ -102,6 +103,24 @@ 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 @@ -129,8 +148,8 @@ function AttachmentCarousel({report, reportActions, parentReportActions, source,

<AttachmentCarouselPager
items={attachments}
initialPage={page}
activeSource={activeSource}
renderItem={renderItem}
initialIndex={page}
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,7 +12,21 @@ const propTypes = {
...withLocalizePropTypes,
};

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

Expand Down
8 changes: 8 additions & 0 deletions src/components/Attachments/AttachmentView/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ function AttachmentView({
translate,
isFocused,
isUsedInCarousel,
isSingleCarouselItem,
carouselItemIndex,
carouselActiveItemIndex,
isUsedInAttachmentModal,
isWorkspaceAvatar,
maybeIcon,
Expand Down Expand Up @@ -144,6 +147,8 @@ function AttachmentView({
isFocused={isFocused}
isAuthTokenRequired={isAuthTokenRequired}
encryptedSourceUrl={encryptedSourceUrl}
carouselItemIndex={carouselItemIndex}
carouselActiveItemIndex={carouselActiveItemIndex}
onPress={onPress}
onToggleKeyboard={onToggleKeyboard}
onLoadComplete={() => !loadComplete && setLoadComplete(true)}
Expand Down Expand Up @@ -172,6 +177,9 @@ function AttachmentView({
loadComplete={loadComplete}
isFocused={isFocused}
isUsedInCarousel={isUsedInCarousel}
isSingleCarouselItem={isSingleCarouselItem}
carouselItemIndex={carouselItemIndex}
carouselActiveItemIndex={carouselActiveItemIndex}
isImage={isImage}
onPress={onPress}
onError={() => {
Expand Down
17 changes: 16 additions & 1 deletion src/components/Attachments/AttachmentView/propTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,18 @@ 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 @@ -27,8 +39,11 @@ const attachmentViewDefaultProps = {
name: '',
},
isFocused: false,
isSingleElement: false,
isUsedInCarousel: false,
isSingleCarouselItem: false,
carouselItemIndex: 0,
carouselActiveItemIndex: 0,
isSingleElement: false,
isUsedInAttachmentModal: false,
onPress: undefined,
onScaleChanged: () => {},
Expand Down
17 changes: 16 additions & 1 deletion src/components/ImageView/index.native.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,28 @@ 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}: ImageViewProps) {
function ImageView({
isAuthTokenRequired = false,
url,
style,
zoomRange = DEFAULT_ZOOM_RANGE,
onError,
isUsedInCarousel = false,
isSingleCarouselItem = false,
carouselItemIndex = 0,
carouselActiveItemIndex = 0,
}: ImageViewProps) {
const hasSiblingCarouselItems = isUsedInCarousel && !isSingleCarouselItem;

return (
<Lightbox
uri={url}
zoomRange={zoomRange}
isAuthTokenRequired={isAuthTokenRequired}
onError={onError}
index={carouselItemIndex}
activeIndex={carouselActiveItemIndex}
hasSiblingCarouselItems={hasSiblingCarouselItems}
style={style}
/>
);
Expand Down
12 changes: 12 additions & 0 deletions src/components/ImageView/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,18 @@ 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 e609383

Please sign in to comment.