From 713ca6562e7b398c1c5d9e7b54e29a1632aabafa Mon Sep 17 00:00:00 2001 From: Anton Gunnarsson Date: Mon, 15 Dec 2025 13:58:00 +0000 Subject: [PATCH] Merged in fix/book-674-select-hotel-infinite-loop (pull request #3351) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fix(BOOK-674): Refactor how we handle hotel filters * Refactor hotel filters store to URL state * Rename hotel filter store Approved-by: Joakim Jäderberg --- .../lib/components/HotelCardListing/index.tsx | 11 ++- .../Filters/FilterAndSortModal/index.tsx | 23 ++---- .../SelectHotel/Filters/HotelFilter/index.tsx | 66 +--------------- .../SelectHotel/Filters/useHotelFilters.ts | 76 +++++++++++++++++++ .../SelectHotel/HotelCount/index.tsx | 4 +- .../SelectHotel/NoAvailabilityAlert.tsx | 7 +- .../SelectHotelMapContent/index.tsx | 9 ++- .../lib/hooks/useInitializeFiltersFromUrl.ts | 18 ----- .../booking-flow/lib/stores/hotel-filters.ts | 29 ------- .../lib/stores/hotel-result-count.ts | 16 ++++ 10 files changed, 120 insertions(+), 139 deletions(-) create mode 100644 packages/booking-flow/lib/components/SelectHotel/Filters/useHotelFilters.ts delete mode 100644 packages/booking-flow/lib/hooks/useInitializeFiltersFromUrl.ts delete mode 100644 packages/booking-flow/lib/stores/hotel-filters.ts create mode 100644 packages/booking-flow/lib/stores/hotel-result-count.ts diff --git a/packages/booking-flow/lib/components/HotelCardListing/index.tsx b/packages/booking-flow/lib/components/HotelCardListing/index.tsx index affa384ce..cc9b49b87 100644 --- a/packages/booking-flow/lib/components/HotelCardListing/index.tsx +++ b/packages/booking-flow/lib/components/HotelCardListing/index.tsx @@ -24,9 +24,10 @@ import { BookingCodeFilterEnum, useBookingCodeFilterStore, } from "../../stores/bookingCode-filter" -import { useHotelFilterStore } from "../../stores/hotel-filters" +import { useHotelResultCountStore } from "../../stores/hotel-result-count" import { useHotelsMapStore } from "../../stores/hotels-map" import { HotelDetailsSidePeek } from "../HotelDetailsSidePeek" +import { useHotelFilters } from "../SelectHotel/Filters/useHotelFilters" import { DEFAULT_SORT } from "../SelectHotel/HotelSorter" import { getSortedHotels } from "./utils" @@ -57,8 +58,10 @@ export default function HotelCardListing({ const intl = useIntl() const isUserLoggedIn = useIsLoggedIn() const searchParams = useSearchParams() - const activeFilters = useHotelFilterStore((state) => state.activeFilters) - const setResultCount = useHotelFilterStore((state) => state.setResultCount) + const [activeFilters] = useHotelFilters(null) + const setResultCount = useHotelResultCountStore( + (state) => state.setResultCount + ) const { activeHotel, activate, disengage, engage } = useHotelsMapStore() const { showBackToTop, scrollToTop } = useScrollToTop({ threshold: 490 }) const activeCardRef = useRef(null) @@ -150,7 +153,7 @@ export default function HotelCardListing({ if (type === HotelCardListingTypeEnum.PageListing) { setResultCount(hotels.length, unfilteredHotelCount) } - }, [hotels, setResultCount, type, unfilteredHotelCount]) + }, [hotels.length, setResultCount, type, unfilteredHotelCount]) function isHotelActiveInMapView(hotelName: string): boolean { return ( diff --git a/packages/booking-flow/lib/components/SelectHotel/Filters/FilterAndSortModal/index.tsx b/packages/booking-flow/lib/components/SelectHotel/Filters/FilterAndSortModal/index.tsx index 070f02e74..ffad6d0e4 100644 --- a/packages/booking-flow/lib/components/SelectHotel/Filters/FilterAndSortModal/index.tsx +++ b/packages/booking-flow/lib/components/SelectHotel/Filters/FilterAndSortModal/index.tsx @@ -20,11 +20,11 @@ import { IconButton } from "@scandic-hotels/design-system/IconButton" import { MaterialIcon } from "@scandic-hotels/design-system/Icons/MaterialIcon" import { Typography } from "@scandic-hotels/design-system/Typography" -import useInitializeFiltersFromUrl from "../../../../hooks/useInitializeFiltersFromUrl" import { SortOrder } from "../../../../misc/sortOrder" -import { useHotelFilterStore } from "../../../../stores/hotel-filters" +import { useHotelResultCountStore } from "../../../../stores/hotel-result-count" import { DEFAULT_SORT } from "../../HotelSorter" import FilterContent from "../FilterContent" +import { useHotelFilters } from "../useHotelFilters" import styles from "./filterAndSortModal.module.css" @@ -46,17 +46,16 @@ export default function FilterAndSortModal({ }: FilterAndSortModalProps) { const intl = useIntl() - useInitializeFiltersFromUrl() const searchParams = useSearchParams() const pathname = usePathname() - const { resultCount, setFilters, activeFilters, unfilteredResultCount } = - useHotelFilterStore((state) => ({ + const { resultCount, unfilteredResultCount } = useHotelResultCountStore( + (state) => ({ resultCount: state.resultCount, - setFilters: state.setFilters, - activeFilters: state.activeFilters, unfilteredResultCount: state.unfilteredResultCount, - })) + }) + ) + const [activeFilters, { setFilters }] = useHotelFilters(filters) const [sort, setSort] = useState(searchParams.get("sort") ?? DEFAULT_SORT) @@ -116,14 +115,6 @@ export default function FilterAndSortModal({ } const newSearchParams = new URLSearchParams(searchParams) - - const values = selectedFilters.join(",") - if (values === "") { - newSearchParams.delete("filters") - } else { - newSearchParams.set("filters", values) - } - newSearchParams.set("sort", sort) window.history.replaceState( diff --git a/packages/booking-flow/lib/components/SelectHotel/Filters/HotelFilter/index.tsx b/packages/booking-flow/lib/components/SelectHotel/Filters/HotelFilter/index.tsx index 37df296ac..26eb7054f 100644 --- a/packages/booking-flow/lib/components/SelectHotel/Filters/HotelFilter/index.tsx +++ b/packages/booking-flow/lib/components/SelectHotel/Filters/HotelFilter/index.tsx @@ -1,13 +1,7 @@ "use client" -import { usePathname, useSearchParams } from "next/navigation" -import { useCallback, useEffect } from "react" - -import { trackEvent } from "@scandic-hotels/tracking/base" - -import useInitializeFiltersFromUrl from "../../../../hooks/useInitializeFiltersFromUrl" -import { useHotelFilterStore } from "../../../../stores/hotel-filters" import FilterContent from "../FilterContent" +import { useHotelFilters } from "../useHotelFilters" import type { CategorizedHotelFilters } from "../../../../types" @@ -17,63 +11,7 @@ type HotelFiltersProps = { } export default function HotelFilter({ className, filters }: HotelFiltersProps) { - const searchParams = useSearchParams() - const pathname = usePathname() - useInitializeFiltersFromUrl() - - const { toggleFilter, activeFilters } = useHotelFilterStore((state) => ({ - toggleFilter: state.toggleFilter, - activeFilters: state.activeFilters, - })) - - const trackFiltersEvent = useCallback(() => { - const facilityMap = new Map( - filters.facilityFilters.map((f) => [f.id.toString(), f.name]) - ) - const surroundingsMap = new Map( - filters.surroundingsFilters.map((f) => [f.id.toString(), f.name]) - ) - - const hotelFacilitiesFilter = activeFilters - .filter((id) => facilityMap.has(id)) - .map((id) => facilityMap.get(id)) - .join(",") - - const hotelSurroundingsFilter = activeFilters - .filter((id) => surroundingsMap.has(id)) - .map((id) => surroundingsMap.get(id)) - .join(",") - - trackEvent({ - event: "filterUsed", - filter: { - filtersUsed: `Filters values - hotelfacilities:${hotelFacilitiesFilter}|hotelsurroundings:${hotelSurroundingsFilter}`, - }, - }) - }, [activeFilters, filters.facilityFilters, filters.surroundingsFilters]) - - // Update the URL when the filters changes - useEffect(() => { - const newSearchParams = new URLSearchParams(searchParams) - const values = activeFilters.join(",") - - if (values === "") { - newSearchParams.delete("filters") - } else { - newSearchParams.set("filters", values) - } - - if (values !== searchParams.get("filters")) { - if (values) { - trackFiltersEvent() - } - window.history.replaceState( - null, - "", - `${pathname}?${newSearchParams.toString()}` - ) - } - }, [activeFilters, pathname, searchParams, trackFiltersEvent]) + const [activeFilters, { toggleFilter }] = useHotelFilters(filters) return ( { + if (!filters) { + // This handles a usage behavior where filterIds are read but never set, in which case we don't pass filters + // to this hook. While filters are only used for tracking purposes, this prevents us from incorrectly + // updating filterIds without providing the filters and hence getting the correct tracking data. + logger.warn( + "Updating filters are not supported when filters parameter is null in useHotelFilters hook" + ) + return + } + + setFilterIds(filterIds.length > 0 ? filterIds : null) + trackFiltersEvent({ + activeFilterIds: filterIds, + filters, + }) + } + + const toggleFilter = (filterId: string) => { + const newFiltersIds = filterIds?.includes(filterId) + ? filterIds.filter((id) => id !== filterId) + : [...(filterIds || []), filterId] + + update(newFiltersIds) + } + + const activeFilters = filterIds || [] + return [activeFilters, { toggleFilter, setFilters: update }] as const +} + +const trackFiltersEvent = ({ + activeFilterIds, + filters, +}: { + activeFilterIds: string[] + filters: CategorizedHotelFilters +}) => { + const facilityMap = new Map( + filters.facilityFilters.map((f) => [f.id.toString(), f.name]) + ) + const surroundingsMap = new Map( + filters.surroundingsFilters.map((f) => [f.id.toString(), f.name]) + ) + + const hotelFacilitiesFilter = activeFilterIds + .filter((id) => facilityMap.has(id)) + .map((id) => facilityMap.get(id)) + .join(",") + + const hotelSurroundingsFilter = activeFilterIds + .filter((id) => surroundingsMap.has(id)) + .map((id) => surroundingsMap.get(id)) + .join(",") + + trackEvent({ + event: "filterUsed", + filter: { + filtersUsed: `Filters values - hotelfacilities:${hotelFacilitiesFilter}|hotelsurroundings:${hotelSurroundingsFilter}`, + }, + }) +} diff --git a/packages/booking-flow/lib/components/SelectHotel/HotelCount/index.tsx b/packages/booking-flow/lib/components/SelectHotel/HotelCount/index.tsx index a98177846..2779da2fc 100644 --- a/packages/booking-flow/lib/components/SelectHotel/HotelCount/index.tsx +++ b/packages/booking-flow/lib/components/SelectHotel/HotelCount/index.tsx @@ -4,11 +4,11 @@ import { useIntl } from "react-intl" import { Typography } from "@scandic-hotels/design-system/Typography" -import { useHotelFilterStore } from "../../../stores/hotel-filters" +import { useHotelResultCountStore } from "../../../stores/hotel-result-count" export default function HotelCount() { const intl = useIntl() - const resultCount = useHotelFilterStore((state) => state.resultCount) + const resultCount = useHotelResultCountStore((state) => state.resultCount) return ( diff --git a/packages/booking-flow/lib/components/SelectHotel/NoAvailabilityAlert.tsx b/packages/booking-flow/lib/components/SelectHotel/NoAvailabilityAlert.tsx index d30b00e5a..a43c50fc8 100644 --- a/packages/booking-flow/lib/components/SelectHotel/NoAvailabilityAlert.tsx +++ b/packages/booking-flow/lib/components/SelectHotel/NoAvailabilityAlert.tsx @@ -7,7 +7,8 @@ import { alternativeHotels } from "@scandic-hotels/common/constants/routes/hotel import { Alert } from "@scandic-hotels/design-system/Alert" import useLang from "../../hooks/useLang" -import { useHotelFilterStore } from "../../stores/hotel-filters" +import { useHotelResultCountStore } from "../../stores/hotel-result-count" +import { useHotelFilters } from "./Filters/useHotelFilters" import type { Hotel } from "@scandic-hotels/trpc/types/hotel" @@ -31,10 +32,10 @@ export default function NoAvailabilityAlert({ const intl = useIntl() const lang = useLang() - const { resultCount, activeFilters } = useHotelFilterStore((state) => ({ + const { resultCount } = useHotelResultCountStore((state) => ({ resultCount: state.resultCount, - activeFilters: state.activeFilters, })) + const [activeFilters] = useHotelFilters(null) if (activeFilters.length > 0 && resultCount === 0) { return ( diff --git a/packages/booking-flow/lib/components/SelectHotel/SelectHotelMap/SelectHotelMapContent/index.tsx b/packages/booking-flow/lib/components/SelectHotel/SelectHotelMap/SelectHotelMapContent/index.tsx index 6dfe5b62a..150560778 100644 --- a/packages/booking-flow/lib/components/SelectHotel/SelectHotelMap/SelectHotelMapContent/index.tsx +++ b/packages/booking-flow/lib/components/SelectHotel/SelectHotelMap/SelectHotelMapContent/index.tsx @@ -27,12 +27,13 @@ import { BookingCodeFilterEnum, useBookingCodeFilterStore, } from "../../../../stores/bookingCode-filter" -import { useHotelFilterStore } from "../../../../stores/hotel-filters" +import { useHotelResultCountStore } from "../../../../stores/hotel-result-count" import { useHotelsMapStore } from "../../../../stores/hotels-map" import BookingCodeFilter from "../../../BookingCodeFilter" import { getHotelPins } from "../../../HotelCardDialogListing/utils" import { RoomCardSkeleton } from "../../../RoomCardSkeleton/RoomCardSkeleton" import FilterAndSortModal from "../../Filters/FilterAndSortModal" +import { useHotelFilters } from "../../Filters/useHotelFilters" import { type HotelResponse } from "../../helpers" import HotelListing from "../HotelListing" import { getVisibleHotels } from "./utils" @@ -75,8 +76,10 @@ export function SelectHotelMapContent({ const [showSkeleton, setShowSkeleton] = useState(true) const listingContainerRef = useRef(null) - const activeFilters = useHotelFilterStore((state) => state.activeFilters) - const setResultCount = useHotelFilterStore((state) => state.setResultCount) + const [activeFilters] = useHotelFilters(null) + const setResultCount = useHotelResultCountStore( + (state) => state.setResultCount + ) const pointsCurrency = useGetPointsCurrency() const hotelMapStore = useHotelsMapStore() diff --git a/packages/booking-flow/lib/hooks/useInitializeFiltersFromUrl.ts b/packages/booking-flow/lib/hooks/useInitializeFiltersFromUrl.ts deleted file mode 100644 index c71662ebd..000000000 --- a/packages/booking-flow/lib/hooks/useInitializeFiltersFromUrl.ts +++ /dev/null @@ -1,18 +0,0 @@ -import { useSearchParams } from "next/navigation" -import { useEffect } from "react" - -import { useHotelFilterStore } from "../stores/hotel-filters" - -export default function useInitializeFiltersFromUrl() { - const searchParams = useSearchParams() - const setFilters = useHotelFilterStore((state) => state.setFilters) - - useEffect(() => { - const filtersFromUrl = searchParams.get("filters") - if (filtersFromUrl) { - setFilters(filtersFromUrl.split(",")) - } else { - setFilters([]) - } - }, [searchParams, setFilters]) -} diff --git a/packages/booking-flow/lib/stores/hotel-filters.ts b/packages/booking-flow/lib/stores/hotel-filters.ts deleted file mode 100644 index 082eb2ce6..000000000 --- a/packages/booking-flow/lib/stores/hotel-filters.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { create } from "zustand" - -interface HotelFilterState { - activeFilters: string[] - toggleFilter: (filterId: string) => void - setFilters: (filters: string[]) => void - resultCount: number - unfilteredResultCount: number - setResultCount: (count: number, unfilteredCount: number) => void -} - -export const useHotelFilterStore = create((set) => ({ - activeFilters: [], - - setFilters: (filters) => set({ activeFilters: filters }), - - toggleFilter: (filterId: string) => - set((state) => { - const isActive = state.activeFilters.includes(filterId) - const newFilters = isActive - ? state.activeFilters.filter((id) => id !== filterId) - : [...state.activeFilters, filterId] - return { activeFilters: newFilters } - }), - resultCount: 0, - unfilteredResultCount: 0, - setResultCount: (count, unfilteredCount) => - set({ resultCount: count, unfilteredResultCount: unfilteredCount }), -})) diff --git a/packages/booking-flow/lib/stores/hotel-result-count.ts b/packages/booking-flow/lib/stores/hotel-result-count.ts new file mode 100644 index 000000000..ca590c196 --- /dev/null +++ b/packages/booking-flow/lib/stores/hotel-result-count.ts @@ -0,0 +1,16 @@ +import { create } from "zustand" + +interface HotelResultCountState { + resultCount: number + unfilteredResultCount: number + setResultCount: (count: number, unfilteredCount: number) => void +} + +export const useHotelResultCountStore = create( + (set) => ({ + resultCount: 0, + unfilteredResultCount: 0, + setResultCount: (count, unfilteredCount) => + set({ resultCount: count, unfilteredResultCount: unfilteredCount }), + }) +)