diff --git a/.github/workflows/NativePipeline.yml b/.github/workflows/NativePipeline.yml index b08b960a4..8300608c0 100644 --- a/.github/workflows/NativePipeline.yml +++ b/.github/workflows/NativePipeline.yml @@ -350,12 +350,35 @@ jobs: - name: "Overlay built widget mpks" if: ${{ github.event.inputs.workspace != 'js-actions' }} shell: bash + # Located by `find` rather than a fixed-depth glob: upload-artifact strips the least-common + # ancestor of the paths it is given, so the depth here follows whatever else is in the + # artifact. Adding resources-manifest.txt at the repo root moved that ancestor from + # packages/ to the root, every path gained a packages/ prefix, and the old + # 'resources/pluggableWidgets/**' glob silently stopped matching — so no built mpk was + # installed and the e2e suite tested the baseline project's widgets instead. + # + # And it is fatal rather than silent: a widget that was built but not installed makes the + # e2e run test something other than the commit under test, which is worse than not running. run: | - if compgen -G 'resources/pluggableWidgets/**/dist/*/*.mpk' > /dev/null; then - for oldPath in resources/pluggableWidgets/**/dist/*/*.mpk; do - newPath="Native-Mobile-Resources-main/widgets/$(basename "$oldPath")" - mv -f "$oldPath" "$newPath" - done + found=0 + while IFS= read -r oldPath; do + [ -n "$oldPath" ] || continue + found=$((found + 1)) + echo "installing $(basename "$oldPath")" + mv -f "$oldPath" "Native-Mobile-Resources-main/widgets/$(basename "$oldPath")" + done < <(find resources -type f -path '*/dist/*/*.mpk') + + # The scope this job was given, read from the manifest rather than needs.scope: `project` + # does not depend on `scope`, so its outputs are not visible here. Compared as a string + # rather than parsed as JSON: the manifest line is written by echoing an interpolated + # expression, and the shell strips the inner quotes on the way in, so it reads + # [a, b] rather than ["a","b"]. + built=$(sed -n 's/^built widgets: //p' resources/resources-manifest.txt) + echo "mpks installed: ${found}; widgets in scope: ${built:-unknown}" + if [ "$found" -eq 0 ] && [ -n "$built" ] && [ "$built" != "[]" ]; then + echo "::error::Widgets ${built} were built but no .mpk was found under resources/ — the test project would run stale widgets." + find resources -type f | head -50 + exit 1 fi - name: "Register widgets in the test project" # Run unconditionally: update-widgets must sync the project's widget definitions with the @@ -364,17 +387,24 @@ jobs: # then skip it and leave stale Atlas widget defs for the portable-app build. shell: bash run: mx update-widgets --loose-version-check Native-Mobile-Resources-main/NativeComponentsTestProject.mpr + # Same layout-independence as the mpk overlay above: locate the dist dir rather than assume + # its depth in the artifact. Each guard also checks its OWN source dir — nanoflow-actions + # previously tested mobile-resources-native, so it moved either both or neither. - name: "Move mobile-resources" shell: bash run: | - if compgen -G 'resources/jsActions/mobile-resources-native/*' > /dev/null; then - mv -f resources/jsActions/mobile-resources-native/* Native-Mobile-Resources-main/javascriptsource/nativemobileresources/actions/ + src=$(find resources -type d -path '*/mobile-resources-native/dist' | head -1) + if [ -n "$src" ]; then + echo "installing mobile-resources from $src" + mv -f "$src"/* Native-Mobile-Resources-main/javascriptsource/nativemobileresources/actions/ fi - name: "Move nanoflow-actions" shell: bash run: | - if compgen -G 'resources/jsActions/mobile-resources-native/*' > /dev/null; then - mv -f resources/jsActions/nanoflow-actions-native/* Native-Mobile-Resources-main/javascriptsource/nanoflowcommons/actions/ + src=$(find resources -type d -path '*/nanoflow-actions-native/dist' | head -1) + if [ -n "$src" ]; then + echo "installing nanoflow-actions from $src" + mv -f "$src"/* Native-Mobile-Resources-main/javascriptsource/nanoflowcommons/actions/ fi - name: "Build portable app package (self-contained runtime)" # Run this BEFORE the native-packager deploy: portable-app-package re-prepares and diff --git a/maestro/helpers/helpers.sh b/maestro/helpers/helpers.sh index 58e9647b7..66d20946e 100644 --- a/maestro/helpers/helpers.sh +++ b/maestro/helpers/helpers.sh @@ -153,10 +153,12 @@ run_maestro() { return "$status" } -# Move a fault's evidence aside BEFORE the retry, which reuses both paths. +# Move an attempt's evidence aside BEFORE the retry, which reuses both paths and would otherwise +# delete it: start_recording rm -f's the same REC_FILE and run_maestro rm -rf's the same +# DEBUG_RUN_DIR, so a flake that passes on retry left no video and no hierarchy to diagnose from. preserve_fault_artifacts() { local video="${1:-}" - local suffix="driver-fault" + local suffix="${2:-driver-fault}" if [ -n "$video" ] && [ -f "$video" ]; then mv -f "$video" "${video%.mp4}-${suffix}.mp4" 2>/dev/null || true fi @@ -318,7 +320,12 @@ run_tests() { fi else echo "❌ Test failed: $yaml_test_file" + # Capture the path before stop_recording clears REC_FILE. + local failed_video="$REC_FILE" stop_recording keep + # The retry reuses both paths, so a flake that passes on retry would erase the only + # evidence of the failure. Keep this attempt's video and hierarchy under -attempt1. + preserve_fault_artifacts "$failed_video" "attempt1" failed_tests+=("$yaml_test_file") fi completed_tests=$((completed_tests + 1)) diff --git a/packages/pluggableWidgets/intro-screen-native/CHANGELOG.md b/packages/pluggableWidgets/intro-screen-native/CHANGELOG.md index b44994119..92d910ebc 100644 --- a/packages/pluggableWidgets/intro-screen-native/CHANGELOG.md +++ b/packages/pluggableWidgets/intro-screen-native/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased] +### Fixed + +- We fixed an issue where the IntroScreen did not show the slide set by the active slide attribute, and where swiping between slides did not work reliably on slower Android devices. + ## [4.4.1] - 2026-6-10 ### Changed diff --git a/packages/pluggableWidgets/intro-screen-native/e2e/specs/maestro/IntroScreen.yaml b/packages/pluggableWidgets/intro-screen-native/e2e/specs/maestro/IntroScreen.yaml index ed2407fe8..266e733e3 100644 --- a/packages/pluggableWidgets/intro-screen-native/e2e/specs/maestro/IntroScreen.yaml +++ b/packages/pluggableWidgets/intro-screen-native/e2e/specs/maestro/IntroScreen.yaml @@ -17,15 +17,33 @@ appId: "${APP_ID}" timeout: 5000 - assertVisible: text: "Changes: 0" +# Coordinates match Gallery_native_horizontal.yaml, which swipes a horizontal list reliably, and +# keep the two directions symmetric — which `direction:` alone also does, but not visibly. +# +# The wait is the part that matters. A slide change writes the active slide attribute and waits for +# the runtime to hand the value back, so the JS thread is still busy for a moment after the +# assertions above pass. Swiping into that window loses the opening touch-move events, so the list +# lags the finger and can be short of the halfway point when the touch lifts — it then snaps back, +# with every counter correctly still on the old slide. +# +# Duration is deliberately left at the default: a paging list commits on position OR lift-off +# velocity, so a slower drag is worse, not better — same distance travelled, less velocity to carry +# it over, and on a slow device the position term is the part already degraded. +- waitForAnimationToEnd: + timeout: 2000 - swipe: - direction: LEFT + start: 90%, 10% + end: 15%, 10% - extendedWaitUntil: visible: "Active slide: 3" timeout: 5000 - assertVisible: text: "Changes: 1" +- waitForAnimationToEnd: + timeout: 2000 - swipe: - direction: RIGHT + start: 15%, 10% + end: 90%, 10% - extendedWaitUntil: visible: "Active slide: 2" timeout: 5000 @@ -53,6 +71,11 @@ appId: "${APP_ID}" timeout: 5000 - tapOn: text: "NEXT" +# NEXT is what turns into FINISH on the last slide, so tapping straight through races the +# re-render: wait for the slide the button belongs to before reaching for it. +- extendedWaitUntil: + visible: "Active slide: 3" + timeout: 5000 - tapOn: text: "FINISH" - extendedWaitUntil: diff --git a/packages/pluggableWidgets/intro-screen-native/package.json b/packages/pluggableWidgets/intro-screen-native/package.json index 747366def..e24db4818 100644 --- a/packages/pluggableWidgets/intro-screen-native/package.json +++ b/packages/pluggableWidgets/intro-screen-native/package.json @@ -1,7 +1,7 @@ { "name": "intro-screen-native", "widgetName": "IntroScreen", - "version": "4.4.1", + "version": "4.4.2", "license": "Apache-2.0", "repository": { "type": "git", diff --git a/packages/pluggableWidgets/intro-screen-native/src/SwipeableContainer.tsx b/packages/pluggableWidgets/intro-screen-native/src/SwipeableContainer.tsx index c64934610..aee6cee28 100644 --- a/packages/pluggableWidgets/intro-screen-native/src/SwipeableContainer.tsx +++ b/packages/pluggableWidgets/intro-screen-native/src/SwipeableContainer.tsx @@ -2,7 +2,6 @@ import { Fragment, ReactElement, ReactNode, useCallback, useEffect, useRef, useS import { I18nManager, LayoutChangeEvent, - NativeSyntheticEvent, Platform, StyleSheet, Text, @@ -53,8 +52,29 @@ const isAndroidRTL = I18nManager.isRTL && Platform.OS === "android"; const Touchable: React.ComponentType = Platform.OS === "android" ? TouchableNativeFeedback : TouchableOpacity; +// Which slide is on screen is asked of the list rather than inferred from scroll offsets and gesture +// phases. Changing this config after mount is not supported by flash-list, so it is a constant. +const VIEWABILITY_CONFIG = { + // Slides are exactly one window wide, so no two of them can be 60% visible at once: whatever + // passes this threshold is the slide the user is looking at, and there is only ever one. + itemVisiblePercentThreshold: 60, + // waitForInteraction is deliberately NOT set. It sounds like what isUserScrolling does, but it + // is gated on flash-list's own hasInteracted, which is only ever set from onScroll and only + // once isInitialScrollComplete — a flag flipped by a 100ms timer started after the first + // layout. A drag that begins before that timer fires therefore records no interaction, and + // since nothing re-records it once the gesture is over, every report for that whole swipe is + // dropped: the list moves and the widget never hears about it. isUserScrolling makes the same + // distinction from the touch itself, with no timer to lose the race against. + // A slide has to hold the screen this long to count, so positions merely passed through on the + // way somewhere else are never reported as arrivals. + minimumViewTime: 250 +} as const; + +// A value is worth reading whenever there is one, not only when the attribute calls itself Available: +// a refresh can report Loading with the previous value still attached, and treating that as "nothing +// to go on" would answer slide 1. const refreshActiveSlideAttribute = (slides: SlidesType[], activeSlide?: EditableValue): number => { - if (activeSlide && activeSlide.status === ValueStatus.Available && slides && slides.length > 0) { + if (activeSlide && activeSlide.value !== undefined && slides && slides.length > 0) { const slide = Number(activeSlide.value) - 1; if (slide < 0) { return 0; @@ -69,9 +89,52 @@ const refreshActiveSlideAttribute = (slides: SlidesType[], activeSlide?: Editabl export const SwipeableContainer = (props: SwipeableContainerProps): ReactElement => { const [width, setWidth] = useState(0); const [height, setHeight] = useState(0); - const [activeIndex, setActiveIndex] = useState(0); + const [activeIndex, setActiveIndex] = useState(() => refreshActiveSlideAttribute(props.slides, props.activeSlide)); const flashList = useRef>(null); - const isInitializing = useRef(true); + // A value written to the attribute round-trips through the runtime, so it arrives back one or + // more renders later. Until it does, the attribute still reads the slide we just left. + const pendingWrite = useRef<{ replaced: number } | null>(null); + // The list re-applies initialScrollIndex from current props for a short window after it mounts, + // so passing live state there lets it overrule our own scrollToOffset. Freeze the slide the + // list opens on instead. + const initialIndex = useRef(activeIndex); + // Only a scroll the user is driving may report a slide change. flash-list's own + // waitForInteraction cannot do this job: it computes viewability from its commit effect without + // consulting that flag, so on a slow device the list is still sitting at offset 0 when reports + // begin and slide 1 is announced as an arrival, losing the slide the widget was asked to open + // on — and worse, its interaction flag is itself gated on a 100ms timer, so it can swallow a + // real swipe outright. Touch is the one signal that cannot be produced by the list scrolling + // itself, and it needs no timer. + const isUserScrolling = useRef(false); + // An attribute is not handed over already loaded: on a fresh page it arrives Loading with no + // value and the real one follows a render or more later. The list opens on initialScrollIndex and + // flash-list applies that at most once — from a commit effect, behind a flag it sets true inside a + // 100ms timer, with no retry afterwards. So whatever the attribute could answer at mount is where + // the list stays: mounting while it is still Loading opens on slide 1 and leaves it there, while + // the dots and buttons follow the value that arrives moments later. The two then disagree for + // good, and the user's first swipe is spent bringing the content into line instead of moving a + // slide. + // + // Unavailable is not worth waiting for — no value is coming, so slide 1 is the right answer + // rather than a blank wait. + const activeSlidePending = + props.activeSlide?.status === ValueStatus.Loading && props.activeSlide.value === undefined; + // Only ever goes false to true, so reading it while rendering is safe. + const listMounted = useRef(false); + + // Until the list is mounted the slide it should open on is still free to change, so keep the + // frozen value current for as long as it is unused — including on the render where the attribute + // finally arrives, which is the one that mounts the list. + if (!listMounted.current && !activeSlidePending) { + initialIndex.current = refreshActiveSlideAttribute(props.slides, props.activeSlide); + if (initialIndex.current !== activeIndex) { + // The buttons and dots read activeIndex, so move it along with the frozen value rather + // than leaving them on slide 1 for the render that mounts the list. Skipped while the + // attribute is still pending: it can only answer slide 1 then, and writing that back + // every render would fight anything else that moved the index in the meantime. + setActiveIndex(initialIndex.current); + } + } const rtlSafeIndex = useCallback( (i: number): number => (isAndroidRTL ? props.slides.length - 1 - i : i), @@ -81,7 +144,7 @@ export const SwipeableContainer = (props: SwipeableContainerProps): ReactElement const goToSlide = useCallback( (pageNum: number) => { setActiveIndex(pageNum); - if (flashList && flashList.current) { + if (width > 0 && flashList && flashList.current) { flashList.current.scrollToOffset({ offset: rtlSafeIndex(pageNum) * width }); @@ -91,19 +154,23 @@ export const SwipeableContainer = (props: SwipeableContainerProps): ReactElement ); useEffect(() => { + if (!width || props.activeSlide?.status !== ValueStatus.Available) { + return; + } const slide = refreshActiveSlideAttribute(props.slides, props.activeSlide); - if (width && props.activeSlide?.status === ValueStatus.Available && slide !== activeIndex) { - goToSlide(slide); - if (isInitializing.current) { - if (isInitializing.current) { - // Use requestAnimationFrame twice to wait for the next frame after scroll. - requestAnimationFrame(() => { - requestAnimationFrame(() => { - isInitializing.current = false; - }); - }); - } + const pending = pendingWrite.current; + if (pending) { + if (slide === pending.replaced) { + // Our own write has not come back yet: the attribute is still reporting the slide we + // navigated away from. Acting on it would scroll straight back and, through + // onSlideChange, overwrite the value we just wrote. + return; } + // Either the write arrived, or something else wrote in the meantime — and that value wins. + pendingWrite.current = null; + } + if (slide !== activeIndex) { + goToSlide(slide); } }, [props.activeSlide, activeIndex, width, props.slides, goToSlide]); @@ -190,6 +257,7 @@ export const SwipeableContainer = (props: SwipeableContainerProps): ReactElement const onSlideChange = useCallback( (newIndex: number, lastIndex: number): void => { if (props.activeSlide && !props.activeSlide.readOnly) { + pendingWrite.current = { replaced: lastIndex }; props.activeSlide.setValue(new Big(newIndex + 1)); } if (props.onSlideChange) { @@ -315,24 +383,39 @@ export const SwipeableContainer = (props: SwipeableContainerProps): ReactElement ); }; - const onMomentumScrollEnd = useCallback( - (event: NativeSyntheticEvent) => { - const offset = event.nativeEvent.contentOffset.x; - const newIndex = rtlSafeIndex(Math.round(offset / width)); - if (newIndex === activeIndex) { + // A touch has landed on the list, so scrolling from here is the user's doing and the slide it + // settles on is a real slide change. Nothing the list does on its own — opening scroll, + // scrollToOffset from goToSlide, an offset correction — passes through here. + const onScrollBeginDrag = useCallback(() => { + isUserScrolling.current = true; + }, []); + + // Which slide is showing comes from the list, not from arithmetic on scroll offsets: flash-list + // reports viewability from its own scroll handling, so a slide that arrives without a momentum + // phase — a drag lifted with no velocity — is reported like any other, and the widget no longer + // has to work out for itself which gesture phase ends a swipe. It only has to know whether the + // scroll being reported is one the user asked for. + const onViewableItemsChanged = useCallback( + ({ viewableItems }: { viewableItems: Array<{ index: number | null }> }) => { + // Reports also arrive while the list is opening on initialScrollIndex, before it has + // reached that offset — announcing the slide it is passing rather than the one it was + // asked for. Acting on those overwrote the active slide attribute with slide 1. + if (!isUserScrolling.current) { return; } - - if (isInitializing.current) { - setActiveIndex(newIndex); + const visible = viewableItems.find(token => token.index !== null); + if (!visible || visible.index === null) { + return; + } + const newIndex = rtlSafeIndex(visible.index); + if (newIndex === activeIndex) { return; } - const lastIndex = activeIndex; setActiveIndex(newIndex); onSlideChange(newIndex, lastIndex); }, - [activeIndex, width, rtlSafeIndex, onSlideChange] + [activeIndex, rtlSafeIndex, onSlideChange] ); /** @@ -353,26 +436,58 @@ export const SwipeableContainer = (props: SwipeableContainerProps): ReactElement [width, height] ); + // Slides are sized from the measured width, so mount the list only once it is known: earlier + // lays every slide out at width zero, stacking them all on the first page. Wait for the active + // slide attribute for the same reason — the list opens on initialScrollIndex and never re-applies + // it, so mounting before the value is known opens it on the wrong slide and leaves it there. + // + // The wait is for the opening value only, which is why it ends for good once the list is up. + // Every write to the attribute sends it back to Loading with no value while the runtime applies + // it, and taking the list down again there would unmount the slide just navigated to and reopen + // on the one it started on — the slide would spring back on every swipe. + const showList = width > 0 && (listMounted.current || !activeSlidePending); + + // Latched on commit rather than during render: a render React discards must not freeze the slide + // the list opens on, since nothing has mounted to hold it. + useEffect(() => { + if (showList) { + listMounted.current = true; + } + }, [showList]); + return ( - "screen_key_" + index} - importantForAccessibility="no" - /> + {showList ? ( + "screen_key_" + index} + importantForAccessibility="no" + /> + ) : ( + + )} {renderPagination()} ); diff --git a/packages/pluggableWidgets/intro-screen-native/src/__tests__/IntroScreen.notch.spec.tsx b/packages/pluggableWidgets/intro-screen-native/src/__tests__/IntroScreen.notch.spec.tsx index cfba23b8e..81b46f4ca 100644 --- a/packages/pluggableWidgets/intro-screen-native/src/__tests__/IntroScreen.notch.spec.tsx +++ b/packages/pluggableWidgets/intro-screen-native/src/__tests__/IntroScreen.notch.spec.tsx @@ -1,4 +1,4 @@ -import { render, act } from "@testing-library/react-native"; +import { render, act, fireEvent, RenderAPI } from "@testing-library/react-native"; import { IntroScreen } from "../IntroScreen"; import { IntroScreenProps } from "../../typings/IntroScreenProps"; import { IntroScreenStyle } from "../ui/Styles"; @@ -16,6 +16,14 @@ jest.mock("@react-native-async-storage/async-storage", () => ({ setItem: jest.fn().mockResolvedValue(null) })); +// The slides are sized from the measured width, so the list only mounts after a layout pass. The +// test renderer never lays out, so the layout event has to be dispatched by hand. +const layout = (component: RenderAPI, name: string): void => { + fireEvent(component.getByTestId(name), "layout", { + nativeEvent: { layout: { width: 400, height: 800 } } + }); +}; + describe("Intro Screen", () => { let defaultProps: IntroScreenProps; @@ -39,6 +47,7 @@ describe("Intro Screen", () => { it("renders", () => { const component = render(); + layout(component, "intro-screen-notch-test"); expect(component.toJSON()).toMatchSnapshot(); }); @@ -46,11 +55,13 @@ describe("Intro Screen", () => { const component = render( ); + layout(component, "intro-screen-notch-test"); expect(component.toJSON()).toMatchSnapshot(); }); it("renders with 2 bottom button", () => { const component = render(); + layout(component, "intro-screen-notch-test"); expect(component.toJSON()).toMatchSnapshot(); }); @@ -61,6 +72,7 @@ describe("Intro Screen", () => { activeSlideAttribute={new EditableValueBuilder().withValue(new Big(1)).build()} /> ); + layout(component, "intro-screen-notch-test"); expect(component.toJSON()).toMatchSnapshot(); }); @@ -68,6 +80,7 @@ describe("Intro Screen", () => { const component = render(); // Wait for async storage to resolve await act(async () => {}); + layout(component, "intro-screen-notch-test"); expect(component.toJSON()).toMatchSnapshot(); }); }); diff --git a/packages/pluggableWidgets/intro-screen-native/src/__tests__/IntroScreen.spec.tsx b/packages/pluggableWidgets/intro-screen-native/src/__tests__/IntroScreen.spec.tsx index 4046ec82c..23deb4731 100644 --- a/packages/pluggableWidgets/intro-screen-native/src/__tests__/IntroScreen.spec.tsx +++ b/packages/pluggableWidgets/intro-screen-native/src/__tests__/IntroScreen.spec.tsx @@ -1,4 +1,4 @@ -import { render, act } from "@testing-library/react-native"; +import { render, act, fireEvent, RenderAPI } from "@testing-library/react-native"; import { IntroScreen } from "../IntroScreen"; import { IntroScreenProps } from "../../typings/IntroScreenProps"; import { IntroScreenStyle } from "../ui/Styles"; @@ -16,6 +16,13 @@ jest.mock("@react-native-async-storage/async-storage", () => ({ setItem: jest.fn().mockResolvedValue(null) })); +// The list only mounts after a layout pass, and the test renderer never lays out. +const layout = (component: RenderAPI, name: string): void => { + fireEvent(component.getByTestId(name), "layout", { + nativeEvent: { layout: { width: 400, height: 800 } } + }); +}; + describe("Intro Screen", () => { let defaultProps: IntroScreenProps; @@ -44,6 +51,7 @@ describe("Intro Screen", () => { it("renders", () => { const component = render(); + layout(component, "intro-screen-test"); expect(component.toJSON()).toMatchSnapshot(); }); @@ -51,11 +59,13 @@ describe("Intro Screen", () => { const component = render( ); + layout(component, "intro-screen-test"); expect(component.toJSON()).toMatchSnapshot(); }); it("renders with 2 bottom button", () => { const component = render(); + layout(component, "intro-screen-test"); expect(component.toJSON()).toMatchSnapshot(); }); @@ -66,6 +76,7 @@ describe("Intro Screen", () => { activeSlideAttribute={new EditableValueBuilder().withValue(new Big(1)).build()} /> ); + layout(component, "intro-screen-test"); expect(component.toJSON()).toMatchSnapshot(); }); @@ -73,6 +84,323 @@ describe("Intro Screen", () => { const component = render(); // Wait for async storage to resolve await act(async () => {}); + layout(component, "intro-screen-test"); expect(component.toJSON()).toMatchSnapshot(); }); + + describe("active slide attribute", () => { + const threeSlides = [ + { name: "Page 1", content: }, + { name: "Page 2", content: }, + { name: "Page 3", content: } + ]; + + // Which slide is showing is reported by the list, not worked out from scroll offsets, so a + // swipe arrives here as a viewability report naming the index that settled on screen. + // Reports are only acted on once a touch has driven the list, so a swipe is a drag followed + // by the report — see "ignores the list's own opening scroll". + const reportViewable = (component: RenderAPI, index: number): void => { + const list = component.getByTestId("intro-screen-test"); + fireEvent(list, "scrollBeginDrag"); + fireEvent(list, "viewableItemsChanged", { + viewableItems: [{ index, isViewable: true }], + changed: [{ index, isViewable: true }] + }); + }; + + it("reports the slide the list says is showing", () => { + const activeSlideAttribute = new EditableValueBuilder().withValue(new Big(1)).build(); + const component = render( + + ); + layout(component, "intro-screen-test"); + + reportViewable(component, 1); + + // Indices are zero-based, the attribute is one-based. + expect(activeSlideAttribute.setValue).toHaveBeenCalledWith(new Big(2)); + }); + + it("reports a swipe made from the slide the attribute already named", () => { + // The regression this guards, seen on an emulator: the widget used to keep its own + // "still initializing" latch and clear it only when the attribute disagreed with the + // slide it had opened on. Starting in agreement meant the latch was never cleared, so + // the first real swipe was swallowed — and worse, the sync effect then scrolled the list + // back, so the slide the user had swiped to silently reverted. Viewability reporting has + // no such latch of its own: flash-list withholds reports until the first interaction and + // then reports every settled slide. + const activeSlideAttribute = new EditableValueBuilder().withValue(new Big(1)).build(); + const component = render( + + ); + layout(component, "intro-screen-test"); + + reportViewable(component, 1); + + expect(activeSlideAttribute.setValue).toHaveBeenCalledTimes(1); + expect(activeSlideAttribute.setValue).toHaveBeenCalledWith(new Big(2)); + // The buttons follow the slide too: on the middle slide the pagination offers Previous, + // which the first slide it opened on does not. + expect(component.queryByTestId("intro-screen-test$buttonPrevious")).not.toBeNull(); + }); + + it("does not report the slide that is already active", () => { + // Viewability is recomputed on every scroll event, so the slide currently showing is + // reported repeatedly while a gesture is in progress. Only arriving somewhere new counts. + const activeSlideAttribute = new EditableValueBuilder().withValue(new Big(2)).build(); + const component = render( + + ); + layout(component, "intro-screen-test"); + + reportViewable(component, 1); + reportViewable(component, 1); + + expect(activeSlideAttribute.setValue).not.toHaveBeenCalled(); + }); + + it("ignores the list's own opening scroll", () => { + // The regression this guards, seen on a CI emulator on both first mount and re-entry: + // the list opens by scrolling to initialScrollIndex, and while it is still on its way it + // reports the slides it passes. Slide 1 was therefore announced as an arrival and + // written to the attribute, so a widget asked to open on slide 2 came up on slide 1 with + // one slide change already counted. + // + // flash-list's own waitForInteraction does not prevent this: it computes viewability + // from its commit effect without consulting that flag at all. It is also unusable in + // the other direction — its interaction flag is gated on a 100ms timer, so a swipe made + // before that timer fires is dropped entirely. Only a touch distinguishes a scroll the + // user asked for, so no drag means no report — see "reports a swipe that begins before + // the list has finished opening". + const activeSlideAttribute = new EditableValueBuilder().withValue(new Big(2)).build(); + const component = render( + + ); + layout(component, "intro-screen-test"); + + // No scrollBeginDrag: this is the list moving itself, not the user moving it. + fireEvent(component.getByTestId("intro-screen-test"), "viewableItemsChanged", { + viewableItems: [{ index: 0, isViewable: true }], + changed: [{ index: 0, isViewable: true }] + }); + + expect(activeSlideAttribute.setValue).not.toHaveBeenCalled(); + // Still on the slide it was asked to open on, so the pagination offers Previous. + expect(component.queryByTestId("intro-screen-test$buttonPrevious")).not.toBeNull(); + }); + + it("reports a swipe that begins before the list has finished opening", () => { + // The regression this guards, seen on a CI emulator and reproducible there: swiping + // within the first moments of mount moved the list to the next slide, but nothing else + // followed — the buttons, the dots and the attribute all stayed on the slide the widget + // opened on, and the flow timed out waiting for the new one. + // + // The cause was viewabilityConfig.waitForInteraction. flash-list gates every report on + // its own hasInteracted, which it records from onScroll and only once + // isInitialScrollComplete — a flag flipped by a 100ms timer started after first layout. + // A drag that lands before that timer fires records no interaction, and nothing + // re-records it afterwards, so the entire swipe is reported to nobody. The gate here + // must therefore be ours alone: the touch says the user is scrolling, whatever + // flash-list's timer has or has not done. + const activeSlideAttribute = new EditableValueBuilder().withValue(new Big(2)).build(); + const component = render( + + ); + layout(component, "intro-screen-test"); + + expect(component.getByTestId("intro-screen-test").props.viewabilityConfig.waitForInteraction).toBe( + undefined + ); + + reportViewable(component, 2); + + expect(activeSlideAttribute.setValue).toHaveBeenCalledWith(new Big(3)); + }); + + it("ignores a viewability report that names no item", () => { + const activeSlideAttribute = new EditableValueBuilder().withValue(new Big(1)).build(); + const component = render( + + ); + layout(component, "intro-screen-test"); + + fireEvent(component.getByTestId("intro-screen-test"), "viewableItemsChanged", { + viewableItems: [], + changed: [] + }); + + expect(activeSlideAttribute.setValue).not.toHaveBeenCalled(); + }); + + it("stays on the new slide while the attribute write is still in flight", () => { + // The regression this guards: setValue round-trips through the runtime, so for a + // render or two the attribute still reports the slide we just left. Syncing from it + // then scrolled straight back and counted the return as another change. + // The builder's setValue applies the value straight away, so stub it out: the point + // here is the window in which the runtime has not applied it yet. + const activeSlideAttribute = new EditableValueBuilder().withValue(new Big(2)).build(); + (activeSlideAttribute.setValue as jest.Mock).mockImplementation(() => undefined); + const component = render( + + ); + layout(component, "intro-screen-test"); + + fireEvent.press(component.getByTestId("intro-screen-test$buttonNext")); + expect(activeSlideAttribute.setValue).toHaveBeenCalledWith(new Big(3)); + + // The stale value arrives on the next render, before the runtime commits the write. + component.update( + + ); + + // Still on the last slide, so its Done button is what the pagination offers. + expect(component.queryByTestId("intro-screen-test$buttonDone")).not.toBeNull(); + expect(component.queryByTestId("intro-screen-test$buttonNext")).toBeNull(); + expect(activeSlideAttribute.setValue).toHaveBeenCalledTimes(1); + }); + + it("mounts the slides only once a width has been measured", () => { + // Rendering the list before onLayout lays every slide out at width 0, which stacks + // them all on the first page and makes the initial scroll offset meaningless. + const component = render( + ().withValue(new Big(3)).build()} + /> + ); + + // Before the layout pass only the measuring placeholder is present, so the list has + // not been given a chance to cache zero-width cell layouts. + expect(component.getByTestId("intro-screen-test").props.initialScrollIndex).toBeUndefined(); + + layout(component, "intro-screen-test"); + + // The first painted frame already targets the attribute's slide, so the list and the + // pagination agree instead of showing slide 1 and then correcting. + expect(component.getByTestId("intro-screen-test").props.initialScrollIndex).toBe(2); + }); + + it("does not re-point the mounted list at the slide navigated to", () => { + // The regression this guards: the list keeps re-applying initialScrollIndex from + // current props for a short window after it mounts. Passing the live active index + // there let that re-apply race our own scrollToOffset, and the list settled back on + // slide 1 while the pagination already showed the last slide's Done button. + const activeSlideAttribute = new EditableValueBuilder().withValue(new Big(2)).build(); + const component = render( + + ); + layout(component, "intro-screen-test"); + + expect(component.getByTestId("intro-screen-test").props.initialScrollIndex).toBe(1); + + fireEvent.press(component.getByTestId("intro-screen-test$buttonNext")); + + // Navigation moves the list with scrollToOffset, so the slide it opened on must not + // follow along: re-applying it would fight that scroll. + expect(component.getByTestId("intro-screen-test").props.initialScrollIndex).toBe(1); + }); + + it("does not let the list hold the previous slide in place", () => { + // The regression this guards: flash-list keeps maintainVisibleContentPosition on by + // default, which anchors the list to the first visible item and scrolls back by however + // far a re-render moved it. Navigating re-renders the slides, so that correction undid + // the scrollToOffset in goToSlide — buttons and dots advanced while the slide on screen + // did not. Only a snapshot covered this prop, and a snapshot update would bury it. + const component = render(); + layout(component, "intro-screen-test"); + + // The list passes this through to its ScrollView, and `disabled` becomes no prop at all. + // Left on, it would arrive here as { minIndexForVisible: 0 } instead. + expect(component.getByTestId("intro-screen-test").props.maintainVisibleContentPosition).toBeUndefined(); + }); + + it("holds the slides back until the attribute has a value to open on", () => { + // The regression this guards, reproduced on a CPU-throttled emulator: the attribute + // arrives Loading with no value and the real one lands a render or two later. Mounting the + // list against the value it could answer then means opening on slide 1 — and flash-list + // applies initialScrollIndex at most once, from a commit effect behind a 100ms timer, with + // no retry — so the content stayed on slide 1 for good while the dots and buttons followed + // the value that arrived moments later. The two disagreed permanently, and the first swipe + // was spent dragging the content into line instead of changing slide. + const activeSlideAttribute = new EditableValueBuilder().isLoading().build(); + const component = render( + + ); + layout(component, "intro-screen-test"); + + // Only the measuring placeholder so far, so the list has not been given a slide to open on. + expect(component.getByTestId("intro-screen-test").props.initialScrollIndex).toBeUndefined(); + + component.update( + ().withValue(new Big(3)).build()} + /> + ); + + // The first frame the list ever draws already targets the slide the attribute named. + expect(component.getByTestId("intro-screen-test").props.initialScrollIndex).toBe(2); + }); + + it("keeps the slides up while a value it already has is refreshing", () => { + // Loading is not on its own a reason to wait: a refresh can arrive with the previous value + // still attached, and there is no need to take the slides down over a value we already have. + const refreshing = new EditableValueBuilder().withValue(new Big(2)).isLoading().build(); + const component = render( + + ); + layout(component, "intro-screen-test"); + + expect(component.getByTestId("intro-screen-test").props.initialScrollIndex).toBe(1); + }); + + it("keeps the slides up while the attribute reloads with no value after a swipe", () => { + // The regression this guards, reproduced on a CPU-throttled emulator: writing to the + // attribute sends it back to Loading with no value at all while the runtime applies the + // write — the same state a fresh page starts in. Waiting there took the list down and + // remounted it on initialScrollIndex, so every slide swiped to sprang back to the one the + // widget opened on, and the return was counted as a second slide change. The wait is for + // the opening value only, so it has to end once the list is up. + const activeSlideAttribute = new EditableValueBuilder().withValue(new Big(2)).build(); + const component = render( + + ); + layout(component, "intro-screen-test"); + + reportViewable(component, 2); + expect(activeSlideAttribute.setValue).toHaveBeenCalledWith(new Big(3)); + + // The write round-trip, as this host reports it: loading, value gone. + component.update( + ().isLoading().build()} + /> + ); + + // Still the same mounted list, still on the slide swiped to — an unmount would take + // initialScrollIndex back to the slide it opened on and lose the swipe. + expect(component.getByTestId("intro-screen-test").props.initialScrollIndex).toBe(1); + expect(component.queryByTestId("intro-screen-test$buttonDone")).not.toBeNull(); + expect(activeSlideAttribute.setValue).toHaveBeenCalledTimes(1); + }); + + it("opens on the first slide when the attribute has no value to give", () => { + // Unavailable is not the same wait as Loading: nothing is on its way, so holding the + // slides back would leave the widget blank for good rather than for a render. + const component = render( + ().isUnavailable().build()} + /> + ); + layout(component, "intro-screen-test"); + + expect(component.getByTestId("intro-screen-test").props.initialScrollIndex).toBe(0); + }); + }); }); diff --git a/packages/pluggableWidgets/intro-screen-native/src/__tests__/__snapshots__/IntroScreen.notch.spec.tsx.snap b/packages/pluggableWidgets/intro-screen-native/src/__tests__/__snapshots__/IntroScreen.notch.spec.tsx.snap index 1e0342d71..fbc2597a0 100644 --- a/packages/pluggableWidgets/intro-screen-native/src/__tests__/__snapshots__/IntroScreen.notch.spec.tsx.snap +++ b/packages/pluggableWidgets/intro-screen-native/src/__tests__/__snapshots__/IntroScreen.notch.spec.tsx.snap @@ -47,30 +47,22 @@ exports[`Intro Screen renders 1`] = ` importantForAccessibility="no" initialScrollIndex={0} keyExtractor={[Function]} - maintainVisibleContentPosition={ - { - "minIndexForVisible": 0, - } - } onLayout={[Function]} - onMomentumScrollEnd={[Function]} onScroll={[Function]} + onScrollBeginDrag={[Function]} + onViewableItemsChanged={[Function]} pagingEnabled={true} scrollEventThrottle={50} showsHorizontalScrollIndicator={false} testID="intro-screen-notch-test" + viewabilityConfig={ + { + "itemVisiblePercentThreshold": 60, + "minimumViewTime": 250, + } + } > - @@ -153,7 +145,7 @@ exports[`Intro Screen renders 1`] = ` "justifyContent": "center", }, { - "width": 0, + "width": 133.33333333333334, }, ] } @@ -202,7 +194,7 @@ exports[`Intro Screen renders 1`] = ` "paddingVertical": 12, }, { - "width": 0, + "width": 133.33333333333334, }, ] } @@ -273,30 +265,22 @@ exports[`Intro Screen renders with 1 bottom button 1`] = ` importantForAccessibility="no" initialScrollIndex={0} keyExtractor={[Function]} - maintainVisibleContentPosition={ - { - "minIndexForVisible": 0, - } - } onLayout={[Function]} - onMomentumScrollEnd={[Function]} onScroll={[Function]} + onScrollBeginDrag={[Function]} + onViewableItemsChanged={[Function]} pagingEnabled={true} scrollEventThrottle={50} showsHorizontalScrollIndicator={false} testID="intro-screen-notch-test" + viewabilityConfig={ + { + "itemVisiblePercentThreshold": 60, + "minimumViewTime": 250, + } + } > - @@ -513,30 +497,22 @@ exports[`Intro Screen renders with 2 bottom button 1`] = ` importantForAccessibility="no" initialScrollIndex={0} keyExtractor={[Function]} - maintainVisibleContentPosition={ - { - "minIndexForVisible": 0, - } - } onLayout={[Function]} - onMomentumScrollEnd={[Function]} onScroll={[Function]} + onScrollBeginDrag={[Function]} + onViewableItemsChanged={[Function]} pagingEnabled={true} scrollEventThrottle={50} showsHorizontalScrollIndicator={false} testID="intro-screen-notch-test" + viewabilityConfig={ + { + "itemVisiblePercentThreshold": 60, + "minimumViewTime": 250, + } + } > - @@ -753,30 +729,22 @@ exports[`Intro Screen renders with active slide attribute 1`] = ` importantForAccessibility="no" initialScrollIndex={0} keyExtractor={[Function]} - maintainVisibleContentPosition={ - { - "minIndexForVisible": 0, - } - } onLayout={[Function]} - onMomentumScrollEnd={[Function]} onScroll={[Function]} + onScrollBeginDrag={[Function]} + onViewableItemsChanged={[Function]} pagingEnabled={true} scrollEventThrottle={50} showsHorizontalScrollIndicator={false} testID="intro-screen-notch-test" + viewabilityConfig={ + { + "itemVisiblePercentThreshold": 60, + "minimumViewTime": 250, + } + } > - @@ -859,7 +827,7 @@ exports[`Intro Screen renders with active slide attribute 1`] = ` "justifyContent": "center", }, { - "width": 0, + "width": 133.33333333333334, }, ] } @@ -908,7 +876,7 @@ exports[`Intro Screen renders with active slide attribute 1`] = ` "paddingVertical": 12, }, { - "width": 0, + "width": 133.33333333333334, }, ] } @@ -979,30 +947,22 @@ exports[`Intro Screen renders with async storage identifier 1`] = ` importantForAccessibility="no" initialScrollIndex={0} keyExtractor={[Function]} - maintainVisibleContentPosition={ - { - "minIndexForVisible": 0, - } - } onLayout={[Function]} - onMomentumScrollEnd={[Function]} onScroll={[Function]} + onScrollBeginDrag={[Function]} + onViewableItemsChanged={[Function]} pagingEnabled={true} scrollEventThrottle={50} showsHorizontalScrollIndicator={false} testID="intro-screen-notch-test" + viewabilityConfig={ + { + "itemVisiblePercentThreshold": 60, + "minimumViewTime": 250, + } + } > - @@ -1085,7 +1045,7 @@ exports[`Intro Screen renders with async storage identifier 1`] = ` "justifyContent": "center", }, { - "width": 0, + "width": 133.33333333333334, }, ] } @@ -1134,7 +1094,7 @@ exports[`Intro Screen renders with async storage identifier 1`] = ` "paddingVertical": 12, }, { - "width": 0, + "width": 133.33333333333334, }, ] } diff --git a/packages/pluggableWidgets/intro-screen-native/src/__tests__/__snapshots__/IntroScreen.spec.tsx.snap b/packages/pluggableWidgets/intro-screen-native/src/__tests__/__snapshots__/IntroScreen.spec.tsx.snap index e923f4fff..01bd790bc 100644 --- a/packages/pluggableWidgets/intro-screen-native/src/__tests__/__snapshots__/IntroScreen.spec.tsx.snap +++ b/packages/pluggableWidgets/intro-screen-native/src/__tests__/__snapshots__/IntroScreen.spec.tsx.snap @@ -47,30 +47,22 @@ exports[`Intro Screen renders 1`] = ` importantForAccessibility="no" initialScrollIndex={0} keyExtractor={[Function]} - maintainVisibleContentPosition={ - { - "minIndexForVisible": 0, - } - } onLayout={[Function]} - onMomentumScrollEnd={[Function]} onScroll={[Function]} + onScrollBeginDrag={[Function]} + onViewableItemsChanged={[Function]} pagingEnabled={true} scrollEventThrottle={50} showsHorizontalScrollIndicator={false} testID="intro-screen-test" + viewabilityConfig={ + { + "itemVisiblePercentThreshold": 60, + "minimumViewTime": 250, + } + } > - @@ -153,7 +145,7 @@ exports[`Intro Screen renders 1`] = ` "justifyContent": "center", }, { - "width": 0, + "width": 133.33333333333334, }, ] } @@ -202,7 +194,7 @@ exports[`Intro Screen renders 1`] = ` "paddingVertical": 12, }, { - "width": 0, + "width": 133.33333333333334, }, ] } @@ -273,30 +265,22 @@ exports[`Intro Screen renders with 1 bottom button 1`] = ` importantForAccessibility="no" initialScrollIndex={0} keyExtractor={[Function]} - maintainVisibleContentPosition={ - { - "minIndexForVisible": 0, - } - } onLayout={[Function]} - onMomentumScrollEnd={[Function]} onScroll={[Function]} + onScrollBeginDrag={[Function]} + onViewableItemsChanged={[Function]} pagingEnabled={true} scrollEventThrottle={50} showsHorizontalScrollIndicator={false} testID="intro-screen-test" + viewabilityConfig={ + { + "itemVisiblePercentThreshold": 60, + "minimumViewTime": 250, + } + } > - @@ -513,30 +497,22 @@ exports[`Intro Screen renders with 2 bottom button 1`] = ` importantForAccessibility="no" initialScrollIndex={0} keyExtractor={[Function]} - maintainVisibleContentPosition={ - { - "minIndexForVisible": 0, - } - } onLayout={[Function]} - onMomentumScrollEnd={[Function]} onScroll={[Function]} + onScrollBeginDrag={[Function]} + onViewableItemsChanged={[Function]} pagingEnabled={true} scrollEventThrottle={50} showsHorizontalScrollIndicator={false} testID="intro-screen-test" + viewabilityConfig={ + { + "itemVisiblePercentThreshold": 60, + "minimumViewTime": 250, + } + } > - @@ -753,30 +729,22 @@ exports[`Intro Screen renders with active slide attribute 1`] = ` importantForAccessibility="no" initialScrollIndex={0} keyExtractor={[Function]} - maintainVisibleContentPosition={ - { - "minIndexForVisible": 0, - } - } onLayout={[Function]} - onMomentumScrollEnd={[Function]} onScroll={[Function]} + onScrollBeginDrag={[Function]} + onViewableItemsChanged={[Function]} pagingEnabled={true} scrollEventThrottle={50} showsHorizontalScrollIndicator={false} testID="intro-screen-test" + viewabilityConfig={ + { + "itemVisiblePercentThreshold": 60, + "minimumViewTime": 250, + } + } > - @@ -859,7 +827,7 @@ exports[`Intro Screen renders with active slide attribute 1`] = ` "justifyContent": "center", }, { - "width": 0, + "width": 133.33333333333334, }, ] } @@ -908,7 +876,7 @@ exports[`Intro Screen renders with active slide attribute 1`] = ` "paddingVertical": 12, }, { - "width": 0, + "width": 133.33333333333334, }, ] } @@ -979,30 +947,22 @@ exports[`Intro Screen renders with async storage identifier 1`] = ` importantForAccessibility="no" initialScrollIndex={0} keyExtractor={[Function]} - maintainVisibleContentPosition={ - { - "minIndexForVisible": 0, - } - } onLayout={[Function]} - onMomentumScrollEnd={[Function]} onScroll={[Function]} + onScrollBeginDrag={[Function]} + onViewableItemsChanged={[Function]} pagingEnabled={true} scrollEventThrottle={50} showsHorizontalScrollIndicator={false} testID="intro-screen-test" + viewabilityConfig={ + { + "itemVisiblePercentThreshold": 60, + "minimumViewTime": 250, + } + } > - @@ -1085,7 +1045,7 @@ exports[`Intro Screen renders with async storage identifier 1`] = ` "justifyContent": "center", }, { - "width": 0, + "width": 133.33333333333334, }, ] } @@ -1134,7 +1094,7 @@ exports[`Intro Screen renders with async storage identifier 1`] = ` "paddingVertical": 12, }, { - "width": 0, + "width": 133.33333333333334, }, ] } diff --git a/packages/pluggableWidgets/intro-screen-native/src/package.xml b/packages/pluggableWidgets/intro-screen-native/src/package.xml index 163c7b253..56fa41861 100644 --- a/packages/pluggableWidgets/intro-screen-native/src/package.xml +++ b/packages/pluggableWidgets/intro-screen-native/src/package.xml @@ -1,6 +1,6 @@ - +