From 45b480f612a0991b5251e5fd7b1d25aa6f98ef4a Mon Sep 17 00:00:00 2001 From: Szymon Zalarski Date: Thu, 12 Feb 2026 17:31:46 +0100 Subject: [PATCH 1/6] Limit fallback image rendering and add priority to reduce carousel load times --- src/components/Image/types.ts | 5 +++++ src/components/Lightbox/index.tsx | 34 +++++++++++++++++++++++++++---- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/components/Image/types.ts b/src/components/Image/types.ts index 7a57250b9848..662787825838 100644 --- a/src/components/Image/types.ts +++ b/src/components/Image/types.ts @@ -27,6 +27,11 @@ type BaseImageProps = { /** The image cache policy */ cachePolicy?: ImagePrefetchOptions['cachePolicy']; + + /** Priorities for completing loads. If more than one load is queued at a time, + * the load with the higher priority will be started first. + * Maps to SDWebImageHighPriority (iOS) and Glide.Priority.IMMEDIATE (Android). */ + priority?: 'low' | 'normal' | 'high' | null; }; type ImageOwnProps = BaseImageProps & { diff --git a/src/components/Lightbox/index.tsx b/src/components/Lightbox/index.tsx index ee6329027e1f..4467bb812a54 100644 --- a/src/components/Lightbox/index.tsx +++ b/src/components/Lightbox/index.tsx @@ -20,6 +20,8 @@ import CONST from '@src/CONST'; import type {Dimensions} from '@src/types/utils/Layout'; import NUMBER_OF_CONCURRENT_LIGHTBOXES from './numberOfConcurrentLightboxes'; +const FALLBACK_OFFSET = 2; + const cachedImageDimensions = new Map(); type LightboxProps = Pick & { @@ -144,10 +146,21 @@ function Lightbox({attachmentID, isAuthTokenRequired = false, uri, onScaleChange const indexOutOfRange = page > activePage + indexCanvasOffset || page < activePage - indexCanvasOffset; return !indexOutOfRange; }, [activePage, hasSiblingCarouselItems, page]); + + // Limits fallback image rendering to only a few pages around the active page. + // This prevents distant carousel items from queuing unnecessary image downloads, + // which would starve the active image of network bandwidth. + const isFallbackInRange = useMemo(() => { + if (!hasSiblingCarouselItems) { + return true; + } + return Math.abs(page - activePage) <= FALLBACK_OFFSET; + }, [activePage, hasSiblingCarouselItems, page]); + const [isLightboxImageLoaded, setLightboxImageLoaded] = useState(false); const [isLoading, setIsLoading] = useState(true); - const [isFallbackVisible, setFallbackVisible] = useState(!isLightboxVisible); + const [isFallbackVisible, setFallbackVisible] = useState(!isLightboxVisible && isFallbackInRange); const [isFallbackImageLoaded, setFallbackImageLoaded] = useState(false); const previousUri = usePrevious(uri); @@ -214,10 +227,11 @@ function Lightbox({attachmentID, isAuthTokenRequired = false, uri, onScaleChange } // If the carousel item has become inactive and the lightbox is not continued to be rendered, we want to show the fallback image + // but only if the page is within the fallback range to avoid unnecessary image downloads if (!isActive && !isLightboxVisible) { - setFallbackVisible(true); + setFallbackVisible(isFallbackInRange); } - }, [hasSiblingCarouselItems, isActive, isFallbackVisible, isLightboxImageLoaded, isLightboxVisible]); + }, [hasSiblingCarouselItems, isActive, isFallbackInRange, isFallbackVisible, isLightboxImageLoaded, isLightboxVisible]); const scaleChange = useCallback( (scale: number) => { @@ -227,6 +241,16 @@ function Lightbox({attachmentID, isAuthTokenRequired = false, uri, onScaleChange [onScaleChangedContext, onScaleChangedProp], ); + const imagePriority = useMemo(() => { + if (isActive) { + return 'high' as const; + } + if (isLightboxVisible) { + return 'normal' as const; + } + return 'low' as const; + }, [isActive, isLightboxVisible]); + const isALocalFile = isLocalFile(uri); const shouldShowOfflineIndicator = isOffline && !isLoading && !isALocalFile; @@ -257,6 +281,7 @@ function Lightbox({attachmentID, isAuthTokenRequired = false, uri, onScaleChange source={{uri}} style={[contentSize ?? styles.invisibleImage]} isAuthTokenRequired={isAuthTokenRequired} + priority={imagePriority} onError={onError} onLoad={(e) => { updateContentSize(e); @@ -279,13 +304,14 @@ function Lightbox({attachmentID, isAuthTokenRequired = false, uri, onScaleChange )} {/* Keep rendering the image without gestures as fallback if the carousel item is not active and while the lightbox is loading the image */} - {isFallbackVisible && ( + {isFallbackVisible && isFallbackInRange && ( { updateContentSize(e); setFallbackImageLoaded(true); From d610d12fac67c3221cbeb07c21712679f33065ae Mon Sep 17 00:00:00 2001 From: Szymon Zalarski Date: Fri, 13 Feb 2026 08:18:06 +0100 Subject: [PATCH 2/6] Use constant variables for image props --- src/CONST/index.ts | 6 ++++++ src/components/Image/types.ts | 2 +- src/components/Lightbox/index.tsx | 6 +++--- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/CONST/index.ts b/src/CONST/index.ts index ed32f1906578..4a874f7e37be 100755 --- a/src/CONST/index.ts +++ b/src/CONST/index.ts @@ -2201,6 +2201,12 @@ const CONST = { INITIAL: 'initial', }, + IMAGE_LOADING_PRIORITY: { + LOW: 'low', + NORMAL: 'normal', + HIGH: 'high', + }, + FILE_TYPE_REGEX: { // Image MimeTypes allowed by iOS photos app. IMAGE: /\.(jpg|jpeg|png|webp|gif|tiff|bmp|heic|heif)$/, diff --git a/src/components/Image/types.ts b/src/components/Image/types.ts index 662787825838..5dc10f1b90be 100644 --- a/src/components/Image/types.ts +++ b/src/components/Image/types.ts @@ -31,7 +31,7 @@ type BaseImageProps = { /** Priorities for completing loads. If more than one load is queued at a time, * the load with the higher priority will be started first. * Maps to SDWebImageHighPriority (iOS) and Glide.Priority.IMMEDIATE (Android). */ - priority?: 'low' | 'normal' | 'high' | null; + priority?: ValueOf | null; }; type ImageOwnProps = BaseImageProps & { diff --git a/src/components/Lightbox/index.tsx b/src/components/Lightbox/index.tsx index 4467bb812a54..15a7a6bb02b1 100644 --- a/src/components/Lightbox/index.tsx +++ b/src/components/Lightbox/index.tsx @@ -243,12 +243,12 @@ function Lightbox({attachmentID, isAuthTokenRequired = false, uri, onScaleChange const imagePriority = useMemo(() => { if (isActive) { - return 'high' as const; + return CONST.IMAGE_LOADING_PRIORITY.HIGH; } if (isLightboxVisible) { - return 'normal' as const; + return CONST.IMAGE_LOADING_PRIORITY.NORMAL; } - return 'low' as const; + return CONST.IMAGE_LOADING_PRIORITY.LOW; }, [isActive, isLightboxVisible]); const isALocalFile = isLocalFile(uri); From a0b80caeeef1255143f91b91faf6be2a67c1deb8 Mon Sep 17 00:00:00 2001 From: Szymon Zalarski Date: Fri, 13 Feb 2026 11:43:55 +0100 Subject: [PATCH 3/6] PR fixes and test added --- src/components/Lightbox/index.tsx | 28 +++--- tests/unit/LightboxTest.tsx | 144 ++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 15 deletions(-) create mode 100644 tests/unit/LightboxTest.tsx diff --git a/src/components/Lightbox/index.tsx b/src/components/Lightbox/index.tsx index 15a7a6bb02b1..181a3cb6f88f 100644 --- a/src/components/Lightbox/index.tsx +++ b/src/components/Lightbox/index.tsx @@ -24,6 +24,16 @@ const FALLBACK_OFFSET = 2; const cachedImageDimensions = new Map(); +function getImagePriority(isActive: boolean, isLightboxVisible: boolean) { + if (isActive) { + return CONST.IMAGE_LOADING_PRIORITY.HIGH; + } + if (isLightboxVisible) { + return CONST.IMAGE_LOADING_PRIORITY.NORMAL; + } + return CONST.IMAGE_LOADING_PRIORITY.LOW; +} + type LightboxProps = Pick & { /** Whether source url requires authentication */ isAuthTokenRequired?: boolean; @@ -150,12 +160,7 @@ function Lightbox({attachmentID, isAuthTokenRequired = false, uri, onScaleChange // Limits fallback image rendering to only a few pages around the active page. // This prevents distant carousel items from queuing unnecessary image downloads, // which would starve the active image of network bandwidth. - const isFallbackInRange = useMemo(() => { - if (!hasSiblingCarouselItems) { - return true; - } - return Math.abs(page - activePage) <= FALLBACK_OFFSET; - }, [activePage, hasSiblingCarouselItems, page]); + const isFallbackInRange = !hasSiblingCarouselItems || Math.abs(page - activePage) <= FALLBACK_OFFSET; const [isLightboxImageLoaded, setLightboxImageLoaded] = useState(false); const [isLoading, setIsLoading] = useState(true); @@ -241,21 +246,14 @@ function Lightbox({attachmentID, isAuthTokenRequired = false, uri, onScaleChange [onScaleChangedContext, onScaleChangedProp], ); - const imagePriority = useMemo(() => { - if (isActive) { - return CONST.IMAGE_LOADING_PRIORITY.HIGH; - } - if (isLightboxVisible) { - return CONST.IMAGE_LOADING_PRIORITY.NORMAL; - } - return CONST.IMAGE_LOADING_PRIORITY.LOW; - }, [isActive, isLightboxVisible]); + const imagePriority = getImagePriority(isActive, isLightboxVisible); const isALocalFile = isLocalFile(uri); const shouldShowOfflineIndicator = isOffline && !isLoading && !isALocalFile; return ( diff --git a/tests/unit/LightboxTest.tsx b/tests/unit/LightboxTest.tsx new file mode 100644 index 000000000000..50f039ce94ce --- /dev/null +++ b/tests/unit/LightboxTest.tsx @@ -0,0 +1,144 @@ +import {fireEvent, render, screen} from '@testing-library/react-native'; +import React from 'react'; +import type {SharedValue} from 'react-native-reanimated'; +import AttachmentCarouselPagerContext from '@components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext'; +import type {AttachmentCarouselPagerContextValue} from '@components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext'; +import Lightbox from '@components/Lightbox'; +import CONST from '@src/CONST'; +import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; + +const TEST_URI = 'https://example.com/image.png'; + +jest.mock('@components/Image', () => { + const MockReact = require('react'); + const {View} = require('react-native'); + function MockImage({priority, testID, ...props}: {priority?: string; testID?: string}) { + return MockReact.createElement(View, { + testID: testID ?? 'image', + accessibilityHint: priority ?? 'none', + ...props, + }); + } + return { + __esModule: true, + default: MockReact.memo(MockImage), + }; +}); + +jest.mock('@components/MultiGestureCanvas', () => { + const MockReact = require('react'); + const {View} = require('react-native'); + return { + __esModule: true, + default: ({children}: {children: unknown}) => MockReact.createElement(View, {testID: 'multi-gesture-canvas'}, children), + DEFAULT_ZOOM_RANGE: {min: 1, max: 5}, + }; +}); + +function createSharedValue(value: T): SharedValue { + return { + value, + get: jest.fn(() => value), + set: jest.fn(), + addListener: jest.fn(), + removeListener: jest.fn(), + modify: jest.fn(), + } as unknown as SharedValue; +} + +function createPagerItems(count: number) { + return Array.from({length: count}, (_, i) => ({ + source: `https://example.com/image-${i}.png`, + previewSource: `https://example.com/image-${i}-preview.png`, + index: i, + isActive: false, + attachmentID: `attachment-${i}`, + })); +} + +function createContextValue(activePage: number, itemCount: number): AttachmentCarouselPagerContextValue { + return { + pagerItems: createPagerItems(itemCount), + activePage, + isPagerScrolling: createSharedValue(false), + isScrollEnabled: createSharedValue(true), + pagerRef: {current: null}, + onTap: jest.fn(), + onScaleChanged: jest.fn(), + onSwipeDown: jest.fn(), + }; +} + +async function renderLightboxInCarousel(attachmentID: string, uri: string, contextValue: AttachmentCarouselPagerContextValue) { + render( + + + , + ); + + await waitForBatchedUpdatesWithAct(); + + fireEvent(screen.getByTestId('lightbox-wrapper'), 'onLayout', { + nativeEvent: {layout: {width: 400, height: 800}}, + }); + + await waitForBatchedUpdatesWithAct(); +} + +describe('Lightbox', () => { + describe('fallback rendering range', () => { + it('should not render any image for distant pages outside FALLBACK_OFFSET range', async () => { + const contextValue = createContextValue(15, 30); + + await renderLightboxInCarousel('attachment-0', TEST_URI, contextValue); + + expect(screen.queryAllByTestId('image')).toHaveLength(0); + }); + + it('should render fallback image for pages within FALLBACK_OFFSET range', async () => { + const contextValue = createContextValue(15, 30); + + await renderLightboxInCarousel('attachment-13', TEST_URI, contextValue); + + expect(screen.queryAllByTestId('image').length).toBeGreaterThan(0); + }); + + it('should render lightbox image for the active page', async () => { + const contextValue = createContextValue(15, 30); + + await renderLightboxInCarousel('attachment-15', TEST_URI, contextValue); + + expect(screen.queryByTestId('multi-gesture-canvas')).not.toBeNull(); + expect(screen.queryAllByTestId('image').length).toBeGreaterThan(0); + }); + }); + + describe('image priority', () => { + it('should assign HIGH priority to the active page image', async () => { + const contextValue = createContextValue(5, 30); + + await renderLightboxInCarousel('attachment-5', TEST_URI, contextValue); + + const images = screen.queryAllByTestId('image'); + expect(images.length).toBeGreaterThan(0); + images.forEach((image) => { + expect(image.props.accessibilityHint).toBe(CONST.IMAGE_LOADING_PRIORITY.HIGH); + }); + }); + + it('should assign LOW priority to fallback images outside the lightbox window', async () => { + const contextValue = createContextValue(15, 30); + + await renderLightboxInCarousel('attachment-13', TEST_URI, contextValue); + + const images = screen.queryAllByTestId('image'); + expect(images.length).toBeGreaterThan(0); + images.forEach((image) => { + expect(image.props.accessibilityHint).toBe(CONST.IMAGE_LOADING_PRIORITY.LOW); + }); + }); + }); +}); From 2e51b074bc71812f82d17f0af8ad65cec3874861 Mon Sep 17 00:00:00 2001 From: Szymon Zalarski Date: Mon, 16 Feb 2026 07:32:55 +0100 Subject: [PATCH 4/6] Lint and prettier fixes --- tests/unit/LightboxTest.tsx | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/tests/unit/LightboxTest.tsx b/tests/unit/LightboxTest.tsx index 50f039ce94ce..aa9e6fcec90e 100644 --- a/tests/unit/LightboxTest.tsx +++ b/tests/unit/LightboxTest.tsx @@ -1,5 +1,6 @@ import {fireEvent, render, screen} from '@testing-library/react-native'; import React from 'react'; +import type {View as RNView} from 'react-native'; import type {SharedValue} from 'react-native-reanimated'; import AttachmentCarouselPagerContext from '@components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext'; import type {AttachmentCarouselPagerContextValue} from '@components/Attachments/AttachmentCarousel/Pager/AttachmentCarouselPagerContext'; @@ -10,8 +11,8 @@ import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct' const TEST_URI = 'https://example.com/image.png'; jest.mock('@components/Image', () => { - const MockReact = require('react'); - const {View} = require('react-native'); + const MockReact = require('react') as typeof React; + const {View} = require('react-native') as {View: typeof RNView}; function MockImage({priority, testID, ...props}: {priority?: string; testID?: string}) { return MockReact.createElement(View, { testID: testID ?? 'image', @@ -20,17 +21,19 @@ jest.mock('@components/Image', () => { }); } return { + // eslint-disable-next-line @typescript-eslint/naming-convention __esModule: true, default: MockReact.memo(MockImage), }; }); jest.mock('@components/MultiGestureCanvas', () => { - const MockReact = require('react'); - const {View} = require('react-native'); + const MockReact = require('react') as typeof React; + const {View} = require('react-native') as {View: typeof RNView}; return { + // eslint-disable-next-line @typescript-eslint/naming-convention __esModule: true, - default: ({children}: {children: unknown}) => MockReact.createElement(View, {testID: 'multi-gesture-canvas'}, children), + default: ({children}: {children: React.ReactNode}) => MockReact.createElement(View, {testID: 'multi-gesture-canvas'}, children), DEFAULT_ZOOM_RANGE: {min: 1, max: 5}, }; }); @@ -103,7 +106,7 @@ describe('Lightbox', () => { await renderLightboxInCarousel('attachment-13', TEST_URI, contextValue); - expect(screen.queryAllByTestId('image').length).toBeGreaterThan(0); + expect(screen.getAllByTestId('image').length).toBeGreaterThan(0); }); it('should render lightbox image for the active page', async () => { @@ -111,8 +114,8 @@ describe('Lightbox', () => { await renderLightboxInCarousel('attachment-15', TEST_URI, contextValue); - expect(screen.queryByTestId('multi-gesture-canvas')).not.toBeNull(); - expect(screen.queryAllByTestId('image').length).toBeGreaterThan(0); + expect(screen.getByTestId('multi-gesture-canvas')).toBeTruthy(); + expect(screen.getAllByTestId('image').length).toBeGreaterThan(0); }); }); @@ -122,11 +125,11 @@ describe('Lightbox', () => { await renderLightboxInCarousel('attachment-5', TEST_URI, contextValue); - const images = screen.queryAllByTestId('image'); + const images = screen.getAllByTestId('image'); expect(images.length).toBeGreaterThan(0); - images.forEach((image) => { + for (const image of images) { expect(image.props.accessibilityHint).toBe(CONST.IMAGE_LOADING_PRIORITY.HIGH); - }); + } }); it('should assign LOW priority to fallback images outside the lightbox window', async () => { @@ -134,11 +137,11 @@ describe('Lightbox', () => { await renderLightboxInCarousel('attachment-13', TEST_URI, contextValue); - const images = screen.queryAllByTestId('image'); + const images = screen.getAllByTestId('image'); expect(images.length).toBeGreaterThan(0); - images.forEach((image) => { + for (const image of images) { expect(image.props.accessibilityHint).toBe(CONST.IMAGE_LOADING_PRIORITY.LOW); - }); + } }); }); }); From 2d0b2f4a880e2ca18e523a23dd06671f44f316cb Mon Sep 17 00:00:00 2001 From: Szymon Zalarski Date: Mon, 16 Feb 2026 07:40:39 +0100 Subject: [PATCH 5/6] Fix consistency in eslint comments --- tests/unit/LightboxTest.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/LightboxTest.tsx b/tests/unit/LightboxTest.tsx index aa9e6fcec90e..90bf07477481 100644 --- a/tests/unit/LightboxTest.tsx +++ b/tests/unit/LightboxTest.tsx @@ -21,7 +21,7 @@ jest.mock('@components/Image', () => { }); } return { - // eslint-disable-next-line @typescript-eslint/naming-convention + // eslint-disable-next-line @typescript-eslint/naming-convention -- __esModule is required by Jest to properly mock ES modules with default exports __esModule: true, default: MockReact.memo(MockImage), }; @@ -31,7 +31,7 @@ jest.mock('@components/MultiGestureCanvas', () => { const MockReact = require('react') as typeof React; const {View} = require('react-native') as {View: typeof RNView}; return { - // eslint-disable-next-line @typescript-eslint/naming-convention + // eslint-disable-next-line @typescript-eslint/naming-convention -- __esModule is required by Jest to properly mock ES modules with default exports __esModule: true, default: ({children}: {children: React.ReactNode}) => MockReact.createElement(View, {testID: 'multi-gesture-canvas'}, children), DEFAULT_ZOOM_RANGE: {min: 1, max: 5}, From b4e6d3063f1c24624778e8f7c24ac83c147198e9 Mon Sep 17 00:00:00 2001 From: Szymon Zalarski Date: Tue, 17 Feb 2026 07:25:55 +0100 Subject: [PATCH 6/6] Add test cases for normal priority --- tests/unit/LightboxTest.tsx | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/unit/LightboxTest.tsx b/tests/unit/LightboxTest.tsx index 90bf07477481..1f70d2180781 100644 --- a/tests/unit/LightboxTest.tsx +++ b/tests/unit/LightboxTest.tsx @@ -27,6 +27,12 @@ jest.mock('@components/Image', () => { }; }); +jest.mock('@components/Lightbox/numberOfConcurrentLightboxes', () => ({ + // eslint-disable-next-line @typescript-eslint/naming-convention -- __esModule is required by Jest to properly mock ES modules with default exports + __esModule: true, + default: 3, +})); + jest.mock('@components/MultiGestureCanvas', () => { const MockReact = require('react') as typeof React; const {View} = require('react-native') as {View: typeof RNView}; @@ -132,6 +138,18 @@ describe('Lightbox', () => { } }); + it('should assign NORMAL priority to non-active pages within the lightbox visible range', async () => { + const contextValue = createContextValue(5, 30); + + await renderLightboxInCarousel('attachment-4', TEST_URI, contextValue); + + const images = screen.getAllByTestId('image'); + expect(images.length).toBeGreaterThan(0); + for (const image of images) { + expect(image.props.accessibilityHint).toBe(CONST.IMAGE_LOADING_PRIORITY.NORMAL); + } + }); + it('should assign LOW priority to fallback images outside the lightbox window', async () => { const contextValue = createContextValue(15, 30);