From 8b12921e46dcc2232c4a45007bd7b44e9ed129dc Mon Sep 17 00:00:00 2001 From: "robbie.sundstrom" Date: Tue, 7 Jan 2025 09:38:40 -0500 Subject: [PATCH 01/11] hook that refreshes on custom interval --- .../v2/elevator/elevator_closures_list.tsx | 4 +-- assets/src/hooks/v2/use_custom_paging.tsx | 27 +++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 assets/src/hooks/v2/use_custom_paging.tsx diff --git a/assets/src/components/v2/elevator/elevator_closures_list.tsx b/assets/src/components/v2/elevator/elevator_closures_list.tsx index edefc3dce..868c90ee3 100644 --- a/assets/src/components/v2/elevator/elevator_closures_list.tsx +++ b/assets/src/components/v2/elevator/elevator_closures_list.tsx @@ -10,7 +10,7 @@ import { type StationWithClosures, type Closure, } from "Components/v2/elevator/types"; -import useClientPaging from "Hooks/v2/use_client_paging"; +import useCustomPaging from "Hooks/v2/use_custom_paging"; import NormalService from "Images/svgr_bundled/normal-service.svg"; import AccessibilityAlert from "Images/svgr_bundled/accessibility-alert.svg"; @@ -132,7 +132,7 @@ const OutsideClosureList = ({ const [rowCountsPerPage, setRowCountsPerPage] = useState([]); const numPages = Object.keys(rowCountsPerPage).length; - const pageIndex = useClientPaging({ numPages, onFinish, lastUpdate }); + const pageIndex = useCustomPaging({ numPages, onFinish, lastUpdate}); const numOffsetRows = Object.keys(rowCountsPerPage).reduce((acc, key) => { if (parseInt(key) === pageIndex) { diff --git a/assets/src/hooks/v2/use_custom_paging.tsx b/assets/src/hooks/v2/use_custom_paging.tsx new file mode 100644 index 000000000..c03aec8d0 --- /dev/null +++ b/assets/src/hooks/v2/use_custom_paging.tsx @@ -0,0 +1,27 @@ +import { WrappedComponentProps } from "Components/v2/persistent_wrapper"; +import { useEffect, useState } from "react"; + +interface Args extends WrappedComponentProps { + numPages: number; +} + +const useCustomPaging = ({ numPages, onFinish, lastUpdate }: Args) => { + const [pageIndex, setPageIndex] = useState(0); + + useEffect(() => { + const interval = setInterval(() => { + setPageIndex((i) => i + 1); + }, 2000); // 5 seconds + return () => clearInterval(interval); // Cleanup on unmount + }, [pageIndex]); + + useEffect(() => { + if (pageIndex === numPages - 1) { + onFinish(); + } + }, [pageIndex]); + + return pageIndex; +}; + +export default useCustomPaging; From 769534c088eb6d3d3b5eefb548dba9495dd3b3bc Mon Sep 17 00:00:00 2001 From: "robbie.sundstrom" Date: Mon, 13 Jan 2025 16:44:36 -0500 Subject: [PATCH 02/11] Refresh on interval works, but timer resets on refresh --- .../v2/elevator/current_elevator_closed.tsx | 13 ++-- .../v2/elevator/elevator_closures_list.tsx | 21 ++++++- .../src/components/v2/persistent_carousel.tsx | 5 +- .../src/components/v2/persistent_wrapper.tsx | 4 +- assets/src/hooks/v2/use_custom_paging.tsx | 27 -------- assets/src/hooks/v2/use_page_advancer.tsx | 62 +++++++++++++++++++ 6 files changed, 94 insertions(+), 38 deletions(-) delete mode 100644 assets/src/hooks/v2/use_custom_paging.tsx create mode 100644 assets/src/hooks/v2/use_page_advancer.tsx diff --git a/assets/src/components/v2/elevator/current_elevator_closed.tsx b/assets/src/components/v2/elevator/current_elevator_closed.tsx index d107c8f09..f0e94f09c 100644 --- a/assets/src/components/v2/elevator/current_elevator_closed.tsx +++ b/assets/src/components/v2/elevator/current_elevator_closed.tsx @@ -1,17 +1,17 @@ -import React, { ComponentType } from "react"; +import React, { ComponentType, useContext } from "react"; import cx from "classnames"; import Arrow, { Direction } from "Components/v2/arrow"; import makePersistent, { WrappedComponentProps, } from "Components/v2/persistent_wrapper"; import PagingIndicators from "Components/v2/elevator/paging_indicators"; -import useClientPaging from "Hooks/v2/use_client_paging"; import useTextResizer from "Hooks/v2/use_text_resizer"; import CurrentLocationMarker from "Images/svgr_bundled/current-location-marker.svg"; import CurrentLocationBackground from "Images/svgr_bundled/current-location-background.svg"; import NoService from "Images/svgr_bundled/no-service-black.svg"; import ElevatorWayfinding from "Images/svgr_bundled/elevator-wayfinding.svg"; import IsaNegative from "Images/svgr_bundled/isa-negative.svg"; +import usePageAdvancer from "Hooks/v2/use_page_advancer"; type Coordinates = { x: number; @@ -40,10 +40,15 @@ const CurrentElevatorClosed = ({ accessible_path_image_url: accessiblePathImageUrl, accessible_path_image_here_coordinates: accessiblePathImageHereCoordinates, onFinish, - lastUpdate, }: Props) => { const numPages = accessiblePathImageUrl ? 2 : 1; - const pageIndex = useClientPaging({ numPages, onFinish, lastUpdate }); + const { pageIndex } = usePageAdvancer({ + numPages, + cycleInterval: 12000, // 12 seconds + advanceOnDataRefresh: false, + onFinish, + }); + const { ref, size } = useTextResizer({ sizes: ["small", "medium", "large"], maxHeight: 746, diff --git a/assets/src/components/v2/elevator/elevator_closures_list.tsx b/assets/src/components/v2/elevator/elevator_closures_list.tsx index 868c90ee3..baa4905ea 100644 --- a/assets/src/components/v2/elevator/elevator_closures_list.tsx +++ b/assets/src/components/v2/elevator/elevator_closures_list.tsx @@ -1,4 +1,10 @@ -import React, { ComponentType, useLayoutEffect, useRef, useState } from "react"; +import React, { + ComponentType, + useContext, + useLayoutEffect, + useRef, + useState, +} from "react"; import cx from "classnames"; import _ from "lodash"; import RoutePill, { routePillKey } from "Components/v2/departures/route_pill"; @@ -10,9 +16,10 @@ import { type StationWithClosures, type Closure, } from "Components/v2/elevator/types"; -import useCustomPaging from "Hooks/v2/use_custom_paging"; import NormalService from "Images/svgr_bundled/normal-service.svg"; import AccessibilityAlert from "Images/svgr_bundled/accessibility-alert.svg"; +import usePageAdvancer from "Hooks/v2/use_page_advancer"; +import { LastFetchContext } from "../screen_container"; interface ClosureRowProps { station: StationWithClosures; @@ -130,9 +137,17 @@ const OutsideClosureList = ({ // Each index represents a page number and each value represents the number of // rows on the corresponding page index. const [rowCountsPerPage, setRowCountsPerPage] = useState([]); + const lastFetch = useContext(LastFetchContext); const numPages = Object.keys(rowCountsPerPage).length; - const pageIndex = useCustomPaging({ numPages, onFinish, lastUpdate}); + + const { pageIndex, advance } = usePageAdvancer({ + numPages, + cycleInterval: 3000, // 12 seconds + advanceOnDataRefresh: false, + lastUpdate: lastFetch, + onFinish: () => console.log("Data refreshed!"), // TODO: For testing + }); const numOffsetRows = Object.keys(rowCountsPerPage).reduce((acc, key) => { if (parseInt(key) === pageIndex) { diff --git a/assets/src/components/v2/persistent_carousel.tsx b/assets/src/components/v2/persistent_carousel.tsx index b75a3efe7..77f48fd90 100644 --- a/assets/src/components/v2/persistent_carousel.tsx +++ b/assets/src/components/v2/persistent_carousel.tsx @@ -1,6 +1,6 @@ import React, { ComponentType, ReactNode } from "react"; import makePersistent, { WrappedComponentProps } from "./persistent_wrapper"; -import useClientPaging from "Hooks/v2/use_client_paging"; +import usePageAdvancer from "Hooks/v2/use_page_advancer"; interface PageRendererProps { page: T; @@ -19,8 +19,9 @@ const Carousel = ({ onFinish, lastUpdate, }: Props): ReactNode => { - const pageIndex = useClientPaging({ + const { pageIndex, advance } = usePageAdvancer({ numPages: pages.length, + advanceOnDataRefresh: true, onFinish, lastUpdate, }); diff --git a/assets/src/components/v2/persistent_wrapper.tsx b/assets/src/components/v2/persistent_wrapper.tsx index 7ff78eec7..5abfe0370 100644 --- a/assets/src/components/v2/persistent_wrapper.tsx +++ b/assets/src/components/v2/persistent_wrapper.tsx @@ -3,7 +3,7 @@ import { LastFetchContext } from "Components/v2/screen_container"; interface WrappedComponentProps { onFinish: () => void; - lastUpdate: number | null; + lastUpdate?: number | null; } interface Props { @@ -37,7 +37,7 @@ const PersistentWrapper: ComponentType = ({ {...visibleData} onFinish={handleFinished} key={renderKey} - lastUpdate={lastFetch} + lastUpdate={lastFetch} // make this optional - either determined by lastFetch Context or by other interval /> ); }; diff --git a/assets/src/hooks/v2/use_custom_paging.tsx b/assets/src/hooks/v2/use_custom_paging.tsx deleted file mode 100644 index c03aec8d0..000000000 --- a/assets/src/hooks/v2/use_custom_paging.tsx +++ /dev/null @@ -1,27 +0,0 @@ -import { WrappedComponentProps } from "Components/v2/persistent_wrapper"; -import { useEffect, useState } from "react"; - -interface Args extends WrappedComponentProps { - numPages: number; -} - -const useCustomPaging = ({ numPages, onFinish, lastUpdate }: Args) => { - const [pageIndex, setPageIndex] = useState(0); - - useEffect(() => { - const interval = setInterval(() => { - setPageIndex((i) => i + 1); - }, 2000); // 5 seconds - return () => clearInterval(interval); // Cleanup on unmount - }, [pageIndex]); - - useEffect(() => { - if (pageIndex === numPages - 1) { - onFinish(); - } - }, [pageIndex]); - - return pageIndex; -}; - -export default useCustomPaging; diff --git a/assets/src/hooks/v2/use_page_advancer.tsx b/assets/src/hooks/v2/use_page_advancer.tsx new file mode 100644 index 000000000..75673cc79 --- /dev/null +++ b/assets/src/hooks/v2/use_page_advancer.tsx @@ -0,0 +1,62 @@ +import { useState, useEffect, useCallback } from "react"; + +interface UsePageAdvancerProps { + numPages: number; // Total number of pages to cycle through + advanceOnDataRefresh: boolean; // Whether to advance when data is refreshed + cycleInterval?: number; // Time interval for cycling pages (only used if advanceOnDataRefresh is false) + lastUpdate?: number | null; + onFinish: () => void; // Callback +} + +function usePageAdvancer({ + numPages, + cycleInterval, + advanceOnDataRefresh = false, + lastUpdate, + onFinish, +}: UsePageAdvancerProps) { + const [pageIndex, setPageIndex] = useState(0); + const [isFirstRender, setIsFirstRender] = useState(true); + + // Function for page advancement + const advancePage = useCallback(() => { + if (lastUpdate !== null) { + if (isFirstRender) { + setIsFirstRender(false); + } else if (numPages > 1) { + setPageIndex((i) => (i + 1) % numPages); + } else { + onFinish(); + } + } + }, [numPages, lastUpdate]); + + // Function for time-interval-based advancement + const advanceByTime = useCallback(() => { + const intervalId = setInterval(advancePage, cycleInterval); + + return () => clearInterval(intervalId); // Cleanup + }, [advancePage, cycleInterval]); + + // Function for data-refresh-based advancement + const advanceOnRefresh = useCallback(() => { + advancePage(); + }, [lastUpdate]); + + // Choose the appropriate function to return + const advance = advanceOnDataRefresh ? advanceOnRefresh : advanceByTime; + + // Start the appropriate page advancement type + useEffect(() => { + if (!advanceOnDataRefresh && cycleInterval) { + return advanceByTime(); + } else { + return advanceOnRefresh(); + } + }, [lastUpdate, cycleInterval]); + + // TODO: I should maybe remove the advance function since it isn't needed anymore after refactoring + return { pageIndex, advance }; +} + +export default usePageAdvancer; From 9af497f7049e3e400697ef7c30f7bd3b06a82efa Mon Sep 17 00:00:00 2001 From: "robbie.sundstrom" Date: Wed, 15 Jan 2025 11:19:32 -0500 Subject: [PATCH 03/11] Comments and readability in page advancer hook --- assets/src/hooks/v2/use_page_advancer.tsx | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/assets/src/hooks/v2/use_page_advancer.tsx b/assets/src/hooks/v2/use_page_advancer.tsx index 75673cc79..0f2b6abdb 100644 --- a/assets/src/hooks/v2/use_page_advancer.tsx +++ b/assets/src/hooks/v2/use_page_advancer.tsx @@ -1,16 +1,22 @@ import { useState, useEffect, useCallback } from "react"; +/** + * This hook acts as a way for components with multiple pages to advance through their pages + * in one of two ways. + * 1. Interval based advancement (advances page every __ ms) + * 2. Refresh based advancement (advances page with every data refresh) + */ interface UsePageAdvancerProps { numPages: number; // Total number of pages to cycle through advanceOnDataRefresh: boolean; // Whether to advance when data is refreshed - cycleInterval?: number; // Time interval for cycling pages (only used if advanceOnDataRefresh is false) + cycleIntervalMs?: number; // In milliseconds, the interval for cycling pages (only used if advanceOnDataRefresh is false) lastUpdate?: number | null; onFinish: () => void; // Callback } function usePageAdvancer({ numPages, - cycleInterval, + cycleIntervalMs, advanceOnDataRefresh = false, lastUpdate, onFinish, @@ -33,10 +39,10 @@ function usePageAdvancer({ // Function for time-interval-based advancement const advanceByTime = useCallback(() => { - const intervalId = setInterval(advancePage, cycleInterval); + const intervalId = setInterval(advancePage, cycleIntervalMs); return () => clearInterval(intervalId); // Cleanup - }, [advancePage, cycleInterval]); + }, [advancePage, cycleIntervalMs]); // Function for data-refresh-based advancement const advanceOnRefresh = useCallback(() => { @@ -48,12 +54,12 @@ function usePageAdvancer({ // Start the appropriate page advancement type useEffect(() => { - if (!advanceOnDataRefresh && cycleInterval) { + if (!advanceOnDataRefresh && cycleIntervalMs) { return advanceByTime(); } else { return advanceOnRefresh(); } - }, [lastUpdate, cycleInterval]); + }, [lastUpdate, cycleIntervalMs]); // TODO: I should maybe remove the advance function since it isn't needed anymore after refactoring return { pageIndex, advance }; From 7845852418c53d48e002fc3b2ec45016d840eafd Mon Sep 17 00:00:00 2001 From: "robbie.sundstrom" Date: Wed, 15 Jan 2025 16:16:57 -0500 Subject: [PATCH 04/11] Fix edge case in usePageAdvancer + cleanup + set time params --- .../v2/elevator/current_elevator_closed.tsx | 4 +- .../v2/elevator/elevator_closures_list.tsx | 10 +-- assets/src/hooks/v2/use_client_paging.tsx | 33 ------- assets/src/hooks/v2/use_page_advancer.tsx | 90 ++++++++++--------- lib/screens/v2/screen_data/parameters.ex | 2 +- 5 files changed, 56 insertions(+), 83 deletions(-) delete mode 100644 assets/src/hooks/v2/use_client_paging.tsx diff --git a/assets/src/components/v2/elevator/current_elevator_closed.tsx b/assets/src/components/v2/elevator/current_elevator_closed.tsx index f0e94f09c..6b36cf73d 100644 --- a/assets/src/components/v2/elevator/current_elevator_closed.tsx +++ b/assets/src/components/v2/elevator/current_elevator_closed.tsx @@ -42,9 +42,9 @@ const CurrentElevatorClosed = ({ onFinish, }: Props) => { const numPages = accessiblePathImageUrl ? 2 : 1; - const { pageIndex } = usePageAdvancer({ + const pageIndex = usePageAdvancer({ numPages, - cycleInterval: 12000, // 12 seconds + cycleIntervalMs: 12000, // 12 seconds advanceOnDataRefresh: false, onFinish, }); diff --git a/assets/src/components/v2/elevator/elevator_closures_list.tsx b/assets/src/components/v2/elevator/elevator_closures_list.tsx index baa4905ea..650760747 100644 --- a/assets/src/components/v2/elevator/elevator_closures_list.tsx +++ b/assets/src/components/v2/elevator/elevator_closures_list.tsx @@ -1,6 +1,5 @@ import React, { ComponentType, - useContext, useLayoutEffect, useRef, useState, @@ -19,7 +18,6 @@ import { import NormalService from "Images/svgr_bundled/normal-service.svg"; import AccessibilityAlert from "Images/svgr_bundled/accessibility-alert.svg"; import usePageAdvancer from "Hooks/v2/use_page_advancer"; -import { LastFetchContext } from "../screen_container"; interface ClosureRowProps { station: StationWithClosures; @@ -137,16 +135,14 @@ const OutsideClosureList = ({ // Each index represents a page number and each value represents the number of // rows on the corresponding page index. const [rowCountsPerPage, setRowCountsPerPage] = useState([]); - const lastFetch = useContext(LastFetchContext); const numPages = Object.keys(rowCountsPerPage).length; - const { pageIndex, advance } = usePageAdvancer({ + const pageIndex = usePageAdvancer({ numPages, - cycleInterval: 3000, // 12 seconds + cycleIntervalMs: 8000, // 8 seconds advanceOnDataRefresh: false, - lastUpdate: lastFetch, - onFinish: () => console.log("Data refreshed!"), // TODO: For testing + onFinish }); const numOffsetRows = Object.keys(rowCountsPerPage).reduce((acc, key) => { diff --git a/assets/src/hooks/v2/use_client_paging.tsx b/assets/src/hooks/v2/use_client_paging.tsx deleted file mode 100644 index 6053326a8..000000000 --- a/assets/src/hooks/v2/use_client_paging.tsx +++ /dev/null @@ -1,33 +0,0 @@ -import { WrappedComponentProps } from "Components/v2/persistent_wrapper"; -import { useEffect, useState } from "react"; - -interface Args extends WrappedComponentProps { - numPages: number; -} - -const useClientPaging = ({ numPages, onFinish, lastUpdate }: Args) => { - const [pageIndex, setPageIndex] = useState(0); - const [isFirstRender, setIsFirstRender] = useState(true); - - useEffect(() => { - if (lastUpdate != null) { - if (isFirstRender) { - setIsFirstRender(false); - } else if (numPages > 1) { - setPageIndex((i) => i + 1); - } else { - onFinish(); - } - } - }, [lastUpdate]); - - useEffect(() => { - if (pageIndex === numPages - 1) { - onFinish(); - } - }, [pageIndex]); - - return pageIndex; -}; - -export default useClientPaging; diff --git a/assets/src/hooks/v2/use_page_advancer.tsx b/assets/src/hooks/v2/use_page_advancer.tsx index 0f2b6abdb..522ef82f9 100644 --- a/assets/src/hooks/v2/use_page_advancer.tsx +++ b/assets/src/hooks/v2/use_page_advancer.tsx @@ -1,19 +1,19 @@ -import { useState, useEffect, useCallback } from "react"; +import { useState, useEffect, useRef, useCallback } from "react"; -/** - * This hook acts as a way for components with multiple pages to advance through their pages - * in one of two ways. - * 1. Interval based advancement (advances page every __ ms) - * 2. Refresh based advancement (advances page with every data refresh) - */ interface UsePageAdvancerProps { numPages: number; // Total number of pages to cycle through advanceOnDataRefresh: boolean; // Whether to advance when data is refreshed cycleIntervalMs?: number; // In milliseconds, the interval for cycling pages (only used if advanceOnDataRefresh is false) lastUpdate?: number | null; - onFinish: () => void; // Callback + onFinish: () => void; // Callback when cycling completes } +/** + * This hook acts as a way for components with multiple pages to advance + * through their pages in one of two ways: + * 1. Interval-based advancement (advances page every __ ms) + * 2. Refresh-based advancement (advances page with every data refresh) + */ function usePageAdvancer({ numPages, cycleIntervalMs, @@ -22,47 +22,57 @@ function usePageAdvancer({ onFinish, }: UsePageAdvancerProps) { const [pageIndex, setPageIndex] = useState(0); - const [isFirstRender, setIsFirstRender] = useState(true); + const intervalRef = useRef(null); - // Function for page advancement - const advancePage = useCallback(() => { - if (lastUpdate !== null) { - if (isFirstRender) { - setIsFirstRender(false); - } else if (numPages > 1) { - setPageIndex((i) => (i + 1) % numPages); - } else { - onFinish(); - } - } - }, [numPages, lastUpdate]); + // Use refs to keep stable references to numPages and onFinish + const numPagesRef = useRef(numPages); + const onFinishRef = useRef(onFinish); - // Function for time-interval-based advancement - const advanceByTime = useCallback(() => { - const intervalId = setInterval(advancePage, cycleIntervalMs); + useEffect(() => { + numPagesRef.current = numPages; + }, [numPages]); - return () => clearInterval(intervalId); // Cleanup - }, [advancePage, cycleIntervalMs]); + useEffect(() => { + onFinishRef.current = onFinish; + }, [onFinish]); - // Function for data-refresh-based advancement - const advanceOnRefresh = useCallback(() => { - advancePage(); - }, [lastUpdate]); + // Callback that handles changing the state of pageIndex + const advancePage = useCallback(() => { + setPageIndex((prevIndex) => { + const nextIndex = (prevIndex + 1) % numPagesRef.current; + if (nextIndex === 0) onFinishRef.current(); // Call onFinish when cycling completes + return nextIndex; + }); + }, []); - // Choose the appropriate function to return - const advance = advanceOnDataRefresh ? advanceOnRefresh : advanceByTime; + // Start the interval for time-based advancement + const startTimer = useCallback(() => { + if (intervalRef.current) clearInterval(intervalRef.current); // Clear any existing timer + if (cycleIntervalMs) { + intervalRef.current = setInterval(() => { + advancePage(); + }, cycleIntervalMs); + } + }, [cycleIntervalMs, advancePage]); - // Start the appropriate page advancement type + // Cleanup the timer interval when the component unmounts or dependencies change useEffect(() => { if (!advanceOnDataRefresh && cycleIntervalMs) { - return advanceByTime(); - } else { - return advanceOnRefresh(); + startTimer(); + return () => { + if (intervalRef.current) clearInterval(intervalRef.current); + }; } - }, [lastUpdate, cycleIntervalMs]); + }, [advanceOnDataRefresh, cycleIntervalMs, startTimer]); + + // Handle refresh-based advancement + useEffect(() => { + if (advanceOnDataRefresh && lastUpdate !== null) { + advancePage(); + } + }, [lastUpdate]); - // TODO: I should maybe remove the advance function since it isn't needed anymore after refactoring - return { pageIndex, advance }; + return pageIndex; } -export default usePageAdvancer; +export default usePageAdvancer; \ No newline at end of file diff --git a/lib/screens/v2/screen_data/parameters.ex b/lib/screens/v2/screen_data/parameters.ex index 5d429e1d6..d4fe5967e 100644 --- a/lib/screens/v2/screen_data/parameters.ex +++ b/lib/screens/v2/screen_data/parameters.ex @@ -38,7 +38,7 @@ defmodule Screens.V2.ScreenData.Parameters do }, elevator_v2: %Static{ candidate_generator: CandidateGenerator.Elevator, - refresh_rate: 8 + refresh_rate: 30 }, gl_eink_v2: %Static{ audio_active_time: @all_times, From 0d5c8788a7a7239d0ca8106634c5cffc00ddd476 Mon Sep 17 00:00:00 2001 From: "robbie.sundstrom" Date: Wed, 15 Jan 2025 16:17:23 -0500 Subject: [PATCH 05/11] format --- .../components/v2/elevator/current_elevator_closed.tsx | 2 +- .../components/v2/elevator/elevator_closures_list.tsx | 9 ++------- assets/src/hooks/v2/use_page_advancer.tsx | 4 ++-- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/assets/src/components/v2/elevator/current_elevator_closed.tsx b/assets/src/components/v2/elevator/current_elevator_closed.tsx index 6b36cf73d..ab6e77df0 100644 --- a/assets/src/components/v2/elevator/current_elevator_closed.tsx +++ b/assets/src/components/v2/elevator/current_elevator_closed.tsx @@ -1,4 +1,4 @@ -import React, { ComponentType, useContext } from "react"; +import React, { ComponentType } from "react"; import cx from "classnames"; import Arrow, { Direction } from "Components/v2/arrow"; import makePersistent, { diff --git a/assets/src/components/v2/elevator/elevator_closures_list.tsx b/assets/src/components/v2/elevator/elevator_closures_list.tsx index 650760747..784d437a5 100644 --- a/assets/src/components/v2/elevator/elevator_closures_list.tsx +++ b/assets/src/components/v2/elevator/elevator_closures_list.tsx @@ -1,9 +1,4 @@ -import React, { - ComponentType, - useLayoutEffect, - useRef, - useState, -} from "react"; +import React, { ComponentType, useLayoutEffect, useRef, useState } from "react"; import cx from "classnames"; import _ from "lodash"; import RoutePill, { routePillKey } from "Components/v2/departures/route_pill"; @@ -142,7 +137,7 @@ const OutsideClosureList = ({ numPages, cycleIntervalMs: 8000, // 8 seconds advanceOnDataRefresh: false, - onFinish + onFinish, }); const numOffsetRows = Object.keys(rowCountsPerPage).reduce((acc, key) => { diff --git a/assets/src/hooks/v2/use_page_advancer.tsx b/assets/src/hooks/v2/use_page_advancer.tsx index 522ef82f9..b5417710e 100644 --- a/assets/src/hooks/v2/use_page_advancer.tsx +++ b/assets/src/hooks/v2/use_page_advancer.tsx @@ -72,7 +72,7 @@ function usePageAdvancer({ } }, [lastUpdate]); - return pageIndex; + return pageIndex; } -export default usePageAdvancer; \ No newline at end of file +export default usePageAdvancer; From 34db43e7c567a5ee189e0991397433abfb9ef27e Mon Sep 17 00:00:00 2001 From: "robbie.sundstrom" Date: Wed, 15 Jan 2025 16:18:50 -0500 Subject: [PATCH 06/11] reorder imports --- assets/src/components/v2/elevator/current_elevator_closed.tsx | 2 +- assets/src/components/v2/elevator/elevator_closures_list.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/assets/src/components/v2/elevator/current_elevator_closed.tsx b/assets/src/components/v2/elevator/current_elevator_closed.tsx index ab6e77df0..8d7b1ee16 100644 --- a/assets/src/components/v2/elevator/current_elevator_closed.tsx +++ b/assets/src/components/v2/elevator/current_elevator_closed.tsx @@ -5,13 +5,13 @@ import makePersistent, { WrappedComponentProps, } from "Components/v2/persistent_wrapper"; import PagingIndicators from "Components/v2/elevator/paging_indicators"; +import usePageAdvancer from "Hooks/v2/use_page_advancer"; import useTextResizer from "Hooks/v2/use_text_resizer"; import CurrentLocationMarker from "Images/svgr_bundled/current-location-marker.svg"; import CurrentLocationBackground from "Images/svgr_bundled/current-location-background.svg"; import NoService from "Images/svgr_bundled/no-service-black.svg"; import ElevatorWayfinding from "Images/svgr_bundled/elevator-wayfinding.svg"; import IsaNegative from "Images/svgr_bundled/isa-negative.svg"; -import usePageAdvancer from "Hooks/v2/use_page_advancer"; type Coordinates = { x: number; diff --git a/assets/src/components/v2/elevator/elevator_closures_list.tsx b/assets/src/components/v2/elevator/elevator_closures_list.tsx index 784d437a5..aef557786 100644 --- a/assets/src/components/v2/elevator/elevator_closures_list.tsx +++ b/assets/src/components/v2/elevator/elevator_closures_list.tsx @@ -10,9 +10,9 @@ import { type StationWithClosures, type Closure, } from "Components/v2/elevator/types"; +import usePageAdvancer from "Hooks/v2/use_page_advancer"; import NormalService from "Images/svgr_bundled/normal-service.svg"; import AccessibilityAlert from "Images/svgr_bundled/accessibility-alert.svg"; -import usePageAdvancer from "Hooks/v2/use_page_advancer"; interface ClosureRowProps { station: StationWithClosures; From ecfdccbd212885d5f6ec76b2334743c855765073 Mon Sep 17 00:00:00 2001 From: "robbie.sundstrom" Date: Thu, 16 Jan 2025 09:36:09 -0500 Subject: [PATCH 07/11] cleanup --- assets/src/components/v2/elevator/elevator_closures_list.tsx | 1 - assets/src/components/v2/persistent_carousel.tsx | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/assets/src/components/v2/elevator/elevator_closures_list.tsx b/assets/src/components/v2/elevator/elevator_closures_list.tsx index aef557786..480c947e7 100644 --- a/assets/src/components/v2/elevator/elevator_closures_list.tsx +++ b/assets/src/components/v2/elevator/elevator_closures_list.tsx @@ -107,7 +107,6 @@ interface OutsideClosureListProps extends WrappedComponentProps { const OutsideClosureList = ({ stations, stationId, - lastUpdate, onFinish, }: OutsideClosureListProps) => { const ref = useRef(null); diff --git a/assets/src/components/v2/persistent_carousel.tsx b/assets/src/components/v2/persistent_carousel.tsx index 77f48fd90..aabb2ddfa 100644 --- a/assets/src/components/v2/persistent_carousel.tsx +++ b/assets/src/components/v2/persistent_carousel.tsx @@ -19,7 +19,7 @@ const Carousel = ({ onFinish, lastUpdate, }: Props): ReactNode => { - const { pageIndex, advance } = usePageAdvancer({ + const pageIndex = usePageAdvancer({ numPages: pages.length, advanceOnDataRefresh: true, onFinish, From cfd595aab34ad0052459e5c3a26e1502ee55a487 Mon Sep 17 00:00:00 2001 From: "robbie.sundstrom" Date: Thu, 16 Jan 2025 09:46:00 -0500 Subject: [PATCH 08/11] Explicity return undefined when no cleanup is needed --- assets/src/hooks/v2/use_page_advancer.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/assets/src/hooks/v2/use_page_advancer.tsx b/assets/src/hooks/v2/use_page_advancer.tsx index b5417710e..69279e94f 100644 --- a/assets/src/hooks/v2/use_page_advancer.tsx +++ b/assets/src/hooks/v2/use_page_advancer.tsx @@ -63,6 +63,9 @@ function usePageAdvancer({ if (intervalRef.current) clearInterval(intervalRef.current); }; } + // Hooks must return void or a cleanup function, + // so we explicitly return undefined when no cleanup is necessary. + return undefined; }, [advanceOnDataRefresh, cycleIntervalMs, startTimer]); // Handle refresh-based advancement From bb3cee3994da682165862c018289a10cfd738c9e Mon Sep 17 00:00:00 2001 From: "robbie.sundstrom" Date: Thu, 16 Jan 2025 11:06:17 -0500 Subject: [PATCH 09/11] Fix state being set outside wrapper warning --- assets/src/hooks/v2/use_page_advancer.tsx | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/assets/src/hooks/v2/use_page_advancer.tsx b/assets/src/hooks/v2/use_page_advancer.tsx index 69279e94f..7b1744ebe 100644 --- a/assets/src/hooks/v2/use_page_advancer.tsx +++ b/assets/src/hooks/v2/use_page_advancer.tsx @@ -22,28 +22,16 @@ function usePageAdvancer({ onFinish, }: UsePageAdvancerProps) { const [pageIndex, setPageIndex] = useState(0); + // Use refs to keep stable references to numPages and interval without triggering re-renders const intervalRef = useRef(null); - - // Use refs to keep stable references to numPages and onFinish const numPagesRef = useRef(numPages); - const onFinishRef = useRef(onFinish); useEffect(() => { numPagesRef.current = numPages; }, [numPages]); - useEffect(() => { - onFinishRef.current = onFinish; - }, [onFinish]); - // Callback that handles changing the state of pageIndex - const advancePage = useCallback(() => { - setPageIndex((prevIndex) => { - const nextIndex = (prevIndex + 1) % numPagesRef.current; - if (nextIndex === 0) onFinishRef.current(); // Call onFinish when cycling completes - return nextIndex; - }); - }, []); + const advancePage = useCallback(, []); // Start the interval for time-based advancement const startTimer = useCallback(() => { @@ -75,6 +63,11 @@ function usePageAdvancer({ } }, [lastUpdate]); + useEffect(() => { + // Call onFinish when cycling completes + if (pageIndex === 0) onFinish(); + }, [pageIndex]); + return pageIndex; } From 0b25cad335501fb477a2c15ab9160bd4e04e293d Mon Sep 17 00:00:00 2001 From: "robbie.sundstrom" Date: Thu, 16 Jan 2025 17:31:34 -0500 Subject: [PATCH 10/11] cleanup page_advancer hook --- assets/src/hooks/v2/use_page_advancer.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/assets/src/hooks/v2/use_page_advancer.tsx b/assets/src/hooks/v2/use_page_advancer.tsx index 7b1744ebe..50f96c49d 100644 --- a/assets/src/hooks/v2/use_page_advancer.tsx +++ b/assets/src/hooks/v2/use_page_advancer.tsx @@ -22,7 +22,7 @@ function usePageAdvancer({ onFinish, }: UsePageAdvancerProps) { const [pageIndex, setPageIndex] = useState(0); - // Use refs to keep stable references to numPages and interval without triggering re-renders + // Use refs to keep stable references to numPages and interval without triggering re-renders const intervalRef = useRef(null); const numPagesRef = useRef(numPages); @@ -31,7 +31,11 @@ function usePageAdvancer({ }, [numPages]); // Callback that handles changing the state of pageIndex - const advancePage = useCallback(, []); + const advancePage = useCallback(() => { + setPageIndex((prevIndex) => { + return (prevIndex + 1) % numPagesRef.current; + }); + }, []); // Start the interval for time-based advancement const startTimer = useCallback(() => { @@ -64,7 +68,6 @@ function usePageAdvancer({ }, [lastUpdate]); useEffect(() => { - // Call onFinish when cycling completes if (pageIndex === 0) onFinish(); }, [pageIndex]); From c05c36cd21b678af0655840ac66769957e5ac938 Mon Sep 17 00:00:00 2001 From: "robbie.sundstrom" Date: Fri, 17 Jan 2025 17:16:51 -0500 Subject: [PATCH 11/11] Working separate hooks, page resets to 0 on data refresh --- .../v2/elevator/current_elevator_closed.tsx | 8 ++-- .../v2/elevator/elevator_closures_list.tsx | 8 ++-- .../v2/elevator/elevator_constants.tsx | 2 + .../src/components/v2/persistent_carousel.tsx | 5 +-- .../src/components/v2/persistent_wrapper.tsx | 6 +-- ...e_advancer.tsx => use_interval_paging.tsx} | 27 ++++--------- assets/src/hooks/v2/use_refresh_paging.tsx | 40 +++++++++++++++++++ 7 files changed, 62 insertions(+), 34 deletions(-) create mode 100644 assets/src/components/v2/elevator/elevator_constants.tsx rename assets/src/hooks/v2/{use_page_advancer.tsx => use_interval_paging.tsx} (71%) create mode 100644 assets/src/hooks/v2/use_refresh_paging.tsx diff --git a/assets/src/components/v2/elevator/current_elevator_closed.tsx b/assets/src/components/v2/elevator/current_elevator_closed.tsx index 8d7b1ee16..e08a734eb 100644 --- a/assets/src/components/v2/elevator/current_elevator_closed.tsx +++ b/assets/src/components/v2/elevator/current_elevator_closed.tsx @@ -5,13 +5,14 @@ import makePersistent, { WrappedComponentProps, } from "Components/v2/persistent_wrapper"; import PagingIndicators from "Components/v2/elevator/paging_indicators"; -import usePageAdvancer from "Hooks/v2/use_page_advancer"; +import useIntervalPaging from "Hooks/v2/use_interval_paging"; import useTextResizer from "Hooks/v2/use_text_resizer"; import CurrentLocationMarker from "Images/svgr_bundled/current-location-marker.svg"; import CurrentLocationBackground from "Images/svgr_bundled/current-location-background.svg"; import NoService from "Images/svgr_bundled/no-service-black.svg"; import ElevatorWayfinding from "Images/svgr_bundled/elevator-wayfinding.svg"; import IsaNegative from "Images/svgr_bundled/isa-negative.svg"; +import { CURRENT_CLOSED_PAGING_INTERVAL_MS } from "./elevator_constants"; type Coordinates = { x: number; @@ -42,10 +43,9 @@ const CurrentElevatorClosed = ({ onFinish, }: Props) => { const numPages = accessiblePathImageUrl ? 2 : 1; - const pageIndex = usePageAdvancer({ + const pageIndex = useIntervalPaging({ numPages, - cycleIntervalMs: 12000, // 12 seconds - advanceOnDataRefresh: false, + cycleIntervalMs: CURRENT_CLOSED_PAGING_INTERVAL_MS, // 12 seconds onFinish, }); diff --git a/assets/src/components/v2/elevator/elevator_closures_list.tsx b/assets/src/components/v2/elevator/elevator_closures_list.tsx index 480c947e7..c8cad4c4e 100644 --- a/assets/src/components/v2/elevator/elevator_closures_list.tsx +++ b/assets/src/components/v2/elevator/elevator_closures_list.tsx @@ -10,9 +10,10 @@ import { type StationWithClosures, type Closure, } from "Components/v2/elevator/types"; -import usePageAdvancer from "Hooks/v2/use_page_advancer"; +import useIntervalPaging from "Hooks/v2/use_interval_paging"; import NormalService from "Images/svgr_bundled/normal-service.svg"; import AccessibilityAlert from "Images/svgr_bundled/accessibility-alert.svg"; +import { CLOSURE_LIST_PAGING_INTERVAL_MS } from "./elevator_constants"; interface ClosureRowProps { station: StationWithClosures; @@ -132,10 +133,9 @@ const OutsideClosureList = ({ const numPages = Object.keys(rowCountsPerPage).length; - const pageIndex = usePageAdvancer({ + const pageIndex = useIntervalPaging({ numPages, - cycleIntervalMs: 8000, // 8 seconds - advanceOnDataRefresh: false, + cycleIntervalMs: CLOSURE_LIST_PAGING_INTERVAL_MS, onFinish, }); diff --git a/assets/src/components/v2/elevator/elevator_constants.tsx b/assets/src/components/v2/elevator/elevator_constants.tsx new file mode 100644 index 000000000..e01003c38 --- /dev/null +++ b/assets/src/components/v2/elevator/elevator_constants.tsx @@ -0,0 +1,2 @@ +export const CURRENT_CLOSED_PAGING_INTERVAL_MS = 12000; // 12 seconds +export const CLOSURE_LIST_PAGING_INTERVAL_MS = 2000; // 8 seconds diff --git a/assets/src/components/v2/persistent_carousel.tsx b/assets/src/components/v2/persistent_carousel.tsx index aabb2ddfa..9fd51e581 100644 --- a/assets/src/components/v2/persistent_carousel.tsx +++ b/assets/src/components/v2/persistent_carousel.tsx @@ -1,6 +1,6 @@ import React, { ComponentType, ReactNode } from "react"; import makePersistent, { WrappedComponentProps } from "./persistent_wrapper"; -import usePageAdvancer from "Hooks/v2/use_page_advancer"; +import useRefreshPaging from "Hooks/v2/use_refresh_paging"; interface PageRendererProps { page: T; @@ -19,9 +19,8 @@ const Carousel = ({ onFinish, lastUpdate, }: Props): ReactNode => { - const pageIndex = usePageAdvancer({ + const pageIndex = useRefreshPaging({ numPages: pages.length, - advanceOnDataRefresh: true, onFinish, lastUpdate, }); diff --git a/assets/src/components/v2/persistent_wrapper.tsx b/assets/src/components/v2/persistent_wrapper.tsx index 5abfe0370..a84b41c40 100644 --- a/assets/src/components/v2/persistent_wrapper.tsx +++ b/assets/src/components/v2/persistent_wrapper.tsx @@ -3,7 +3,7 @@ import { LastFetchContext } from "Components/v2/screen_container"; interface WrappedComponentProps { onFinish: () => void; - lastUpdate?: number | null; + lastUpdate: number | null; } interface Props { @@ -27,7 +27,7 @@ const PersistentWrapper: ComponentType = ({ useEffect(() => { if (isFinished) { setVisibleData(data); - setRenderKey((n) => n + 1); + setRenderKey((n) => n + 1); // TODO: This should be disabled only for interval based advancing setIsFinished(false); } }, [lastFetch]); @@ -37,7 +37,7 @@ const PersistentWrapper: ComponentType = ({ {...visibleData} onFinish={handleFinished} key={renderKey} - lastUpdate={lastFetch} // make this optional - either determined by lastFetch Context or by other interval + lastUpdate={lastFetch} /> ); }; diff --git a/assets/src/hooks/v2/use_page_advancer.tsx b/assets/src/hooks/v2/use_interval_paging.tsx similarity index 71% rename from assets/src/hooks/v2/use_page_advancer.tsx rename to assets/src/hooks/v2/use_interval_paging.tsx index 50f96c49d..47e64f104 100644 --- a/assets/src/hooks/v2/use_page_advancer.tsx +++ b/assets/src/hooks/v2/use_interval_paging.tsx @@ -1,26 +1,20 @@ import { useState, useEffect, useRef, useCallback } from "react"; -interface UsePageAdvancerProps { +interface UseIntervalPagingProps { numPages: number; // Total number of pages to cycle through - advanceOnDataRefresh: boolean; // Whether to advance when data is refreshed cycleIntervalMs?: number; // In milliseconds, the interval for cycling pages (only used if advanceOnDataRefresh is false) - lastUpdate?: number | null; onFinish: () => void; // Callback when cycling completes } /** * This hook acts as a way for components with multiple pages to advance - * through their pages in one of two ways: - * 1. Interval-based advancement (advances page every __ ms) - * 2. Refresh-based advancement (advances page with every data refresh) + * through their pages on a set time interval. */ -function usePageAdvancer({ +function useIntervalPaging({ numPages, cycleIntervalMs, - advanceOnDataRefresh = false, - lastUpdate, onFinish, -}: UsePageAdvancerProps) { +}: UseIntervalPagingProps) { const [pageIndex, setPageIndex] = useState(0); // Use refs to keep stable references to numPages and interval without triggering re-renders const intervalRef = useRef(null); @@ -49,7 +43,7 @@ function usePageAdvancer({ // Cleanup the timer interval when the component unmounts or dependencies change useEffect(() => { - if (!advanceOnDataRefresh && cycleIntervalMs) { + if (cycleIntervalMs) { startTimer(); return () => { if (intervalRef.current) clearInterval(intervalRef.current); @@ -58,14 +52,7 @@ function usePageAdvancer({ // Hooks must return void or a cleanup function, // so we explicitly return undefined when no cleanup is necessary. return undefined; - }, [advanceOnDataRefresh, cycleIntervalMs, startTimer]); - - // Handle refresh-based advancement - useEffect(() => { - if (advanceOnDataRefresh && lastUpdate !== null) { - advancePage(); - } - }, [lastUpdate]); + }, [cycleIntervalMs, startTimer]); useEffect(() => { if (pageIndex === 0) onFinish(); @@ -74,4 +61,4 @@ function usePageAdvancer({ return pageIndex; } -export default usePageAdvancer; +export default useIntervalPaging; diff --git a/assets/src/hooks/v2/use_refresh_paging.tsx b/assets/src/hooks/v2/use_refresh_paging.tsx new file mode 100644 index 000000000..1bedff27c --- /dev/null +++ b/assets/src/hooks/v2/use_refresh_paging.tsx @@ -0,0 +1,40 @@ +import { WrappedComponentProps } from "Components/v2/persistent_wrapper"; +import { useEffect, useState } from "react"; + +interface UseRefreshPagingProps extends WrappedComponentProps { + numPages: number; +} + +/** + * Enables pagination on data refreshes for components that are wrapped in Persistent Wrapper. + */ +const useRefreshPaging = ({ + numPages, + onFinish, + lastUpdate, +}: UseRefreshPagingProps) => { + const [pageIndex, setPageIndex] = useState(0); + const [isFirstRender, setIsFirstRender] = useState(true); + + useEffect(() => { + if (lastUpdate != null) { + if (isFirstRender) { + setIsFirstRender(false); + } else if (numPages > 1) { + setPageIndex((i) => i + 1); + } else { + onFinish(); + } + } + }, [lastUpdate]); + + useEffect(() => { + if (pageIndex === numPages - 1) { + onFinish(); + } + }, [pageIndex]); + + return pageIndex; +}; + +export default useRefreshPaging;