diff --git a/src/app/Components/Countdown/Ticker.tsx b/src/app/Components/Countdown/Ticker.tsx index a8732873ff5..4da12c55a9e 100644 --- a/src/app/Components/Countdown/Ticker.tsx +++ b/src/app/Components/Countdown/Ticker.tsx @@ -21,15 +21,15 @@ export const durationSections = (duration: Duration, labels: [string, string, st label: labels[0], }, { - time: padWithZero(d.hours), + time: padWithZero(Math.floor(d.hours)), label: labels[1], }, { - time: padWithZero(d.minutes), + time: padWithZero(Math.floor(d.minutes)), label: labels[2], }, { - time: padWithZero(d.seconds), + time: padWithZero(Math.floor(d.seconds)), label: labels[3], }, ] diff --git a/src/app/Components/Countdown/__tests__/Ticker.tests.tsx b/src/app/Components/Countdown/__tests__/Ticker.tests.tsx index c6fca17a695..fcd04e74d50 100644 --- a/src/app/Components/Countdown/__tests__/Ticker.tests.tsx +++ b/src/app/Components/Countdown/__tests__/Ticker.tests.tsx @@ -42,6 +42,18 @@ describe("SimpleTicker", () => { }) ).toBeTruthy() }) + + it("does not render fractional seconds / milliseconds", () => { + // 1s + 789ms — the fractional remainder that used to leak through + const fractional = Duration.fromMillis(1789) + renderWithWrappers() + + expect( + screen.getByText("00d 00h 00m 01s", { + normalizer: getDefaultNormalizer({ collapseWhitespace: false }), + }) + ).toBeTruthy() + }) }) describe("LabeledTicker", () => { diff --git a/src/app/Scenes/HomeView/Sections/HomeViewSectionArtworks.tsx b/src/app/Scenes/HomeView/Sections/HomeViewSectionArtworks.tsx index eaa76d2bb18..6859910de33 100644 --- a/src/app/Scenes/HomeView/Sections/HomeViewSectionArtworks.tsx +++ b/src/app/Scenes/HomeView/Sections/HomeViewSectionArtworks.tsx @@ -33,7 +33,7 @@ import { NoFallback, withSuspense } from "app/utils/hooks/withSuspense" import { isDislikeArtworksEnabledFor } from "app/utils/isDislikeArtworksEnabledFor" import { useMemoizedRandom } from "app/utils/placeholders" import { times } from "lodash" -import { memo, useEffect, useRef } from "react" +import { memo, useEffect, useState } from "react" import { fetchQuery, graphql, useFragment, useLazyLoadQuery } from "react-relay" interface HomeViewSectionArtworksProps extends FlexProps { @@ -64,49 +64,59 @@ export const HomeViewSectionArtworks: React.FC = ( const isRailInViewport = viewableSections.includes(section.internalID) - // Ref so the async `complete` callback reads current visibility, not the value at kick-off. - const isRailInViewportRef = useRef(isRailInViewport) - isRailInViewportRef.current = isRailInViewport + // Set when a live refresh completes; cleared once we've re-fired railViewed for it. Lets us fire + // when the rail is on screen — now, or later when it's scrolled into view. + const [hasPendingRailViewed, setHasPendingRailViewed] = useState(false) - const { onViewableItemsChanged, viewabilityConfig, resetTracking } = useItemsImpressionsTracking({ - // It is important here to tell if the rail is visible or not, because the viewability config - // default behavior, doesn't take into account the fact that the rail could be not visible - // on the screen because it's within a scrollable container. - isInViewport: isRailInViewport && section.trackItemImpressions, - contextModule, - }) + const { onViewableItemsChanged, viewabilityConfig, resetTracking, trackingKey } = + useItemsImpressionsTracking({ + // It is important here to tell if the rail is visible or not, because the viewability config + // default behavior, doesn't take into account the fact that the rail could be not visible + // on the screen because it's within a scrollable container. + isInViewport: isRailInViewport && section.trackItemImpressions, + contextModule, + }) - // On a refetch bump, force-fetch this rail. This effect runs per-section, so every live rail - // (recommended, new works for you, and any future ones in liveSectionIDs) refetches — in view - // or not — so no live rail goes stale. On complete, reset the impression guard and re-fire - // railViewed only if the rail is on screen. + // On a refetch bump, flag that railViewed should re-fire for this refresh (the effect below + // fires it once the rail is on screen — now or when scrolled to), and force-fetch this rail. This + // effect runs per-section, so every live rail (recommended, new works for you, and any future + // ones in liveSectionIDs) refetches — in view or not — so no live rail goes stale. On complete we + // reset the per-item impression guard so items re-track against the fresh data. useEffect(() => { - if (!isLiveRefreshRail || liveRefetchKey === 0) { - return - } + if (isLiveRefreshRail && liveRefetchKey > 0) { + setHasPendingRailViewed(true) - const subscription = fetchQuery( - getRelayEnvironment(), - homeViewSectionArtworksQuery, - { id: section.internalID, enableHidingDislikedArtworks }, - { networkCacheConfig: { force: true } } - ).subscribe({ - complete: () => { - resetTracking() - - if (isRailInViewportRef.current) { - tracking.viewedSection(contextModule, index) - } - }, - error: (error: Error) => { - console.error("Failed to refresh live artworks rail", error) - }, - }) + const subscription = fetchQuery( + getRelayEnvironment(), + homeViewSectionArtworksQuery, + { id: section.internalID, enableHidingDislikedArtworks }, + { networkCacheConfig: { force: true } } + ).subscribe({ + complete: () => { + resetTracking() + }, + error: (error: Error) => { + console.error("Failed to refresh live artworks rail", error) + }, + }) + + return () => subscription.unsubscribe() + } - return () => subscription.unsubscribe() // eslint-disable-next-line react-hooks/exhaustive-deps }, [liveRefetchKey]) + // Re-fire railViewed after a live refresh, once the rail is on screen. Driven by the reactive + // `isRailInViewport` (from viewableSections, which updates on scroll), so a rail that was off + // screen at refresh time still re-fires when it's later scrolled into view. + useEffect(() => { + if (hasPendingRailViewed && isRailInViewport) { + tracking.viewedSection(contextModule, index) + setHasPendingRailViewed(false) + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [hasPendingRailViewed, isRailInViewport]) + let artworks = extractNodes(section.artworksConnection) if (isDislikeArtworksEnabledFor(contextModule)) { @@ -182,6 +192,8 @@ export const HomeViewSectionArtworks: React.FC = ( onArtworkPress={handleOnGridArtworkPress} trackItemImpressions={section.trackItemImpressions} contextModule={contextModule} + // We are using this key to force a re-render to track item impressions again + key={trackingKey} /> ) : ( { setLiveSectionIDs([]) }) - it("re-fires railViewed only after the live refresh completes", () => { - const { env } = renderWithRelay({ + it("re-fires railViewed as soon as the rail is refreshed", () => { + renderWithRelay({ HomeViewSectionArtworks: () => RECOMMENDED_SECTION, }) @@ -437,20 +437,6 @@ describe("HomeViewSectionArtworks", () => { homeViewStoreActions.bumpLiveRefetchKey() }) - // Nothing fires yet — the refreshed data hasn't landed. - expect(mockTrackEvent).not.toHaveBeenCalledWith( - expect.objectContaining({ action: "railViewed" }) - ) - - // Once the forced refetch completes, railViewed fires for the fresh data. - act(() => { - env.mock.resolveMostRecentOperation((operation) => - MockPayloadGenerator.generate(operation, { - HomeViewSectionArtworks: () => RECOMMENDED_SECTION, - }) - ) - }) - expect(mockTrackEvent).toHaveBeenCalledWith({ action: "railViewed", context_module: "newWorksForYouRail", @@ -459,8 +445,7 @@ describe("HomeViewSectionArtworks", () => { }) }) - it("does not re-fire railViewed on refresh when the WTYL rail is off screen", () => { - // The refresh-driven railViewed re-fire only happens while the rail is actually on screen. + it("re-fires railViewed once an off-screen rail is scrolled into view after a refresh", () => { const { env } = renderWithRelay({ HomeViewSectionArtworks: () => RECOMMENDED_SECTION, }) @@ -480,10 +465,17 @@ describe("HomeViewSectionArtworks", () => { ) }) - // railViewed should not fire because the rail is off screen. + // Off screen at refresh time — nothing fires yet. expect(mockTrackEvent).not.toHaveBeenCalledWith( expect.objectContaining({ action: "railViewed" }) ) + + // Scrolled into view afterwards — the pending refresh re-fires railViewed. + act(() => { + homeViewStoreActions.setViewableSections(["home-view-section-recommended-artworks"]) + }) + + expect(mockTrackEvent).toHaveBeenCalledWith(expect.objectContaining({ action: "railViewed" })) }) it("re-fires railViewed on every refresh while the rail is in view", () => { @@ -655,8 +647,8 @@ describe("HomeViewSectionArtworks", () => { setLiveSectionIDs([]) }) - it("re-fires railViewed only after the live refresh completes", () => { - const { env } = renderWithRelay({ + it("re-fires railViewed as soon as the rail is refreshed", () => { + renderWithRelay({ HomeViewSectionArtworks: () => NWFY_SECTION, }) @@ -667,19 +659,6 @@ describe("HomeViewSectionArtworks", () => { homeViewStoreActions.bumpLiveRefetchKey() }) - // Nothing fires yet — the refreshed data hasn't landed. - expect(mockTrackEvent).not.toHaveBeenCalledWith( - expect.objectContaining({ action: "railViewed" }) - ) - - act(() => { - env.mock.resolveMostRecentOperation((operation) => - MockPayloadGenerator.generate(operation, { - HomeViewSectionArtworks: () => NWFY_SECTION, - }) - ) - }) - expect(mockTrackEvent).toHaveBeenCalledWith({ action: "railViewed", context_module: "newWorksForYouRail", diff --git a/src/app/Scenes/HomeView/hooks/useImpressionsTracking.ts b/src/app/Scenes/HomeView/hooks/useImpressionsTracking.ts index 29db08a6269..07c41faa918 100644 --- a/src/app/Scenes/HomeView/hooks/useImpressionsTracking.ts +++ b/src/app/Scenes/HomeView/hooks/useImpressionsTracking.ts @@ -24,6 +24,7 @@ export const useItemsImpressionsTracking = ({ // Use different state management for test vs production const renderedItems = useSharedValue>([]) const [testRenderedItems, setTestRenderedItems] = useState>([]) + const [key, setKey] = useState(0) const trackedItems = useRef>(new Set()).current @@ -106,18 +107,20 @@ export const useItemsImpressionsTracking = ({ runOnJS(trackItems)(currentItems) } }, - [enableItemsViewsTracking, isInViewport, contextScreenOwnerType, contextModule] + [enableItemsViewsTracking, isInViewport, contextScreenOwnerType, contextModule, key] ) // Clears the per-item "already tracked" guard so itemViewed events become // eligible to fire again, e.g. after a forced data refresh of the rail. const resetTracking = useCallback(() => { trackedItems.clear() - }, [trackedItems]) + setKey(key + 1) + }, [trackedItems, key]) return { onViewableItemsChanged, - viewabilityConfig, resetTracking, + trackingKey: key, + viewabilityConfig, } } diff --git a/src/app/Scenes/MyProfile/MyProfileEditForm.tsx b/src/app/Scenes/MyProfile/MyProfileEditForm.tsx index f52772f5893..40402327ff7 100644 --- a/src/app/Scenes/MyProfile/MyProfileEditForm.tsx +++ b/src/app/Scenes/MyProfile/MyProfileEditForm.tsx @@ -209,7 +209,7 @@ export const MyProfileEditForm: React.FC = () => { )}