Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: wait for map load #1077

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

fix: wait for map load #1077

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dfac280
fix: wait for map load
emilyjablonski Jan 31, 2025
df62a2e
refactor: remove tests
emilyjablonski Jan 31, 2025
280dac6
refactor: recenter
emilyjablonski Jan 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sites/public/src/components/listings/ListingsCombined.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@

[class*="loading-overlay__spinner"] {
top: var(--seeds-s20);
color: var(--seeds-color-primary);
Copy link
Collaborator Author

@emilyjablonski emilyjablonski Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: Change spinner color to primary - it was invisible on very first page load for a second on a slow connection before the rest of the list loaded

}
}

Expand Down
163 changes: 79 additions & 84 deletions sites/public/src/components/listings/ListingsCombined.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React from "react"
import { APIProvider } from "@vis.gl/react-google-maps"
import { useJsApiLoader } from "@react-google-maps/api"
import { Listing, ListingMapMarker } from "@bloom-housing/shared-helpers/src/types/backend-swagger"
import CustomSiteFooter from "../shared/CustomSiteFooter"
Expand Down Expand Up @@ -48,114 +47,110 @@ const ListingsCombined = (props: ListingsCombinedProps) => {

const getListingsList = () => {
return (
<APIProvider apiKey={props.googleMapsApiKey}>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: Just moved this up a level

<div className={styles["listings-combined"]}>
<ListingsSearchMetadata
loading={props.loading}
setModalOpen={props.setModalOpen}
filterCount={props.filterCount}
searchResults={props.searchResults}
setListView={props.setListView}
listView={props.listView}
/>
<div
className={`${styles["listings-map-list-container"]} ${styles["listings-map-list-container-list-only"]}`}
>
<div id="listings-list-expanded" className={styles["listings-list-expanded"]}>
<ListingsList
listings={props.searchResults.listings}
currentPage={props.searchResults.currentPage}
lastPage={props.searchResults.lastPage}
onPageChange={props.onPageChange}
loading={getListLoading() || (props.isFirstBoundsLoad && props.isDesktop)}
mapMarkers={props.markers}
/>
</div>
<div>
<CustomSiteFooter />
</div>
<div className={styles["listings-combined"]}>
<ListingsSearchMetadata
loading={props.loading}
setModalOpen={props.setModalOpen}
filterCount={props.filterCount}
searchResults={props.searchResults}
setListView={props.setListView}
listView={props.listView}
/>
<div
className={`${styles["listings-map-list-container"]} ${styles["listings-map-list-container-list-only"]}`}
>
<div id="listings-list-expanded" className={styles["listings-list-expanded"]}>
<ListingsList
listings={props.searchResults.listings}
currentPage={props.searchResults.currentPage}
lastPage={props.searchResults.lastPage}
onPageChange={props.onPageChange}
loading={getListLoading() || (props.isFirstBoundsLoad && props.isDesktop)}
mapMarkers={props.markers}
/>
</div>
<div>
<CustomSiteFooter />
</div>
</div>
</APIProvider>
</div>
)
}

const getListingsMap = () => {
return (
<APIProvider apiKey={props.googleMapsApiKey}>
<div className={styles["listings-combined"]}>
<ListingsSearchMetadata
loading={props.loading}
setModalOpen={props.setModalOpen}
filterCount={props.filterCount}
searchResults={props.searchResults}
setListView={props.setListView}
listView={props.listView}
<div className={styles["listings-combined"]}>
<ListingsSearchMetadata
loading={props.loading}
setModalOpen={props.setModalOpen}
filterCount={props.filterCount}
searchResults={props.searchResults}
setListView={props.setListView}
listView={props.listView}
/>
<div className={styles["listings-map-expanded"]}>
<ListingsMap
listings={props.markers}
googleMapsApiKey={props.googleMapsApiKey}
googleMapsMapId={props.googleMapsMapId}
isMapExpanded={true}
setVisibleMarkers={props.setVisibleMarkers}
visibleMarkers={props.visibleMarkers}
setIsLoading={props.setIsLoading}
searchFilter={props.searchFilter}
isFirstBoundsLoad={props.isFirstBoundsLoad}
setIsFirstBoundsLoad={props.setIsFirstBoundsLoad}
isDesktop={props.isDesktop}
isLoading={props.loading}
/>
<div className={styles["listings-map-expanded"]}>
</div>
</div>
)
}

const getListingsCombined = () => {
return (
<div className={styles["listings-combined"]}>
<ListingsSearchMetadata
loading={props.loading}
setModalOpen={props.setModalOpen}
filterCount={props.filterCount}
searchResults={props.searchResults}
setListView={props.setListView}
listView={props.listView}
/>
<div className={styles["listings-map-list-container"]}>
<div className={styles["listings-map"]}>
<ListingsMap
listings={props.markers}
googleMapsApiKey={props.googleMapsApiKey}
googleMapsMapId={props.googleMapsMapId}
isMapExpanded={true}
isMapExpanded={false}
setVisibleMarkers={props.setVisibleMarkers}
visibleMarkers={props.visibleMarkers}
setIsLoading={props.setIsLoading}
searchFilter={props.searchFilter}
isFirstBoundsLoad={props.isFirstBoundsLoad}
setIsFirstBoundsLoad={props.setIsFirstBoundsLoad}
isDesktop={props.isDesktop}
isLoading={props.loading}
/>
</div>
</div>
</APIProvider>
)
}

const getListingsCombined = () => {
return (
<APIProvider apiKey={props.googleMapsApiKey}>
<div className={styles["listings-combined"]}>
<ListingsSearchMetadata
loading={props.loading}
setModalOpen={props.setModalOpen}
filterCount={props.filterCount}
searchResults={props.searchResults}
setListView={props.setListView}
listView={props.listView}
/>
<div className={styles["listings-map-list-container"]}>
<div className={styles["listings-map"]}>
<ListingsMap
listings={props.markers}
googleMapsApiKey={props.googleMapsApiKey}
googleMapsMapId={props.googleMapsMapId}
isMapExpanded={false}
setVisibleMarkers={props.setVisibleMarkers}
visibleMarkers={props.visibleMarkers}
setIsLoading={props.setIsLoading}
searchFilter={props.searchFilter}
isFirstBoundsLoad={props.isFirstBoundsLoad}
setIsFirstBoundsLoad={props.setIsFirstBoundsLoad}
isDesktop={props.isDesktop}
<div id="listings-outer-container" className={styles["listings-outer-container"]}>
<div id="listings-list" className={styles["listings-list"]}>
<ListingsList
listings={props.searchResults.listings}
currentPage={props.searchResults.currentPage}
lastPage={props.searchResults.lastPage}
loading={getListLoading() || (props.isFirstBoundsLoad && props.isDesktop)}
onPageChange={props.onPageChange}
mapMarkers={props.markers}
/>
</div>
<div id="listings-outer-container" className={styles["listings-outer-container"]}>
<div id="listings-list" className={styles["listings-list"]}>
<ListingsList
listings={props.searchResults.listings}
currentPage={props.searchResults.currentPage}
lastPage={props.searchResults.lastPage}
loading={getListLoading() || (props.isFirstBoundsLoad && props.isDesktop)}
onPageChange={props.onPageChange}
mapMarkers={props.markers}
/>
<CustomSiteFooter />
</div>
<CustomSiteFooter />
</div>
</div>
</div>
</APIProvider>
</div>
)
}

Expand Down
7 changes: 6 additions & 1 deletion sites/public/src/components/listings/ListingsMap.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type ListingsMapProps = {
isFirstBoundsLoad: boolean
setIsFirstBoundsLoad: React.Dispatch<React.SetStateAction<boolean>>
isDesktop: boolean
isLoading: boolean
}

export type MapMarkerData = {
Expand Down Expand Up @@ -82,7 +83,11 @@ const ListingsMap = (props: ListingsMapProps) => {
clickableIcons={false}
>
<MapControl />
<MapRecenter mapMarkers={props.listings} visibleMapMarkers={props.visibleMarkers?.length} />
<MapRecenter
mapMarkers={props.listings}
visibleMapMarkers={props.visibleMarkers?.length}
isLoading={props.isLoading}
/>
<MapClusterer
mapMarkers={markers}
infoWindowIndex={infoWindowIndex}
Expand Down
12 changes: 6 additions & 6 deletions sites/public/src/components/listings/MapClusterer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ export type ListingsMapMarkersProps = {
export const fitBounds = (
map: google.maps.Map,
mapMarkers: MapMarkerData[],
continueIfEmpty?: boolean
continueIfEmpty?: boolean,
setIsFirstBoundsLoad?: React.Dispatch<React.SetStateAction<boolean>>
) => {
const bounds = new window.google.maps.LatLngBounds()

Expand All @@ -50,6 +51,9 @@ export const fitBounds = (
map.setZoom(zoomLevel - 7)
}
}
if (setIsFirstBoundsLoad) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: Move setting the bounds load to false into the set bounds function itself

setIsFirstBoundsLoad(false)
}
}

// Zoom in slowly by recursively setting the zoom level
Expand Down Expand Up @@ -203,11 +207,7 @@ export const MapClusterer = ({
// Only automatically size the map to fit all pins on first map load
if (isFirstBoundsLoad === false) return

fitBounds(map, mapMarkers)

setTimeout(() => {
Copy link
Collaborator Author

@emilyjablonski emilyjablonski Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: Seems like this really shouldn't have been here, I think it was intended to be a setTimeout(0) - this was moved into the set bounds fxn

setIsFirstBoundsLoad(false)
}, 1000)
fitBounds(map, mapMarkers, false, setIsFirstBoundsLoad)

// eslint-disable-next-line react-hooks/exhaustive-deps
}, [clusterer, markers, currentMapMarkers])
Expand Down
20 changes: 18 additions & 2 deletions sites/public/src/components/listings/search/ListingsSearchCombined.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import React, { useState, useEffect, useContext } from "react"
import { UserStatus } from "../../../lib/constants"
import { useMap } from "@vis.gl/react-google-maps"
import { ListingList, pushGtmEvent, AuthContext } from "@bloom-housing/shared-helpers"
import { t } from "@bloom-housing/ui-components"
import {
ListingSearchParams,
generateSearchQuery,
parseSearchString,
} from "../../../lib/listings/search"
import { UserStatus } from "../../../lib/constants"
import { searchListings, searchMapMarkers } from "../../../lib/listings/listing-service"
import styles from "./ListingsSearch.module.scss"
import { ListingsCombined } from "../ListingsCombined"
Expand Down Expand Up @@ -75,8 +76,18 @@ function ListingsSearchCombined(props: ListingsSearchCombinedProps) {
// This can be changed later if needed
const pageSize = 25

const map = useMap()

const search = async (page: number, changingFilter?: boolean) => {
// If a user pans over empty space, reset the listings to empty instead of refetching

const oldMarkersSearch = JSON.stringify(
currentMarkers?.sort((a, b) => a.coordinate.lat - b.coordinate.lat)
)
const newMarkersSearch = JSON.stringify(
visibleMarkers?.sort((a, b) => a.coordinate.lat - b.coordinate.lat)
)

if (visibleMarkers?.length === 0 && !changingFilter) {
setSearchResults({
listings: [],
Expand Down Expand Up @@ -106,6 +117,8 @@ function ListingsSearchCombined(props: ListingsSearchCombinedProps) {
// Don't search the listings if the filter is changing - first search the markers, which will update the listings
if (
(!isFirstBoundsLoad &&
!!map &&
Copy link
Collaborator Author

@emilyjablonski emilyjablonski Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: Only search for listings if the map has loaded (involves both new checks here) - this I cannot reproduce, but is just a suspicion on what may be happening

oldMarkersSearch !== newMarkersSearch &&
!changingFilter &&
(isDesktop || listView) &&
!(visibleMarkers?.length === 0 && changingFilter)) ||
Expand Down Expand Up @@ -144,7 +157,9 @@ function ListingsSearchCombined(props: ListingsSearchCombinedProps) {
})

// On desktop, keep list loading set to true until map is finished with its first load
if (!isFirstBoundsLoad || !isDesktop) setIsLoading(false)
if ((!isFirstBoundsLoad && newMarkersSearch !== undefined && !changingFilter) || !isDesktop) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: Keep the loading state through fully changing the filter

setIsLoading(false)
}

document.getElementById("listings-outer-container")?.scrollTo(0, 0)
document.getElementById("listings-list")?.scrollTo(0, 0)
Expand Down Expand Up @@ -233,6 +248,7 @@ function ListingsSearchCombined(props: ListingsSearchCombinedProps) {
}, [isFirstBoundsLoad])

const onFormSubmit = (params: ListingSearchParams) => {
setIsLoading(true)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: Initiate loading state earlier when the filter is changed

setSearchFilter(params)
}

Expand Down
5 changes: 4 additions & 1 deletion sites/public/src/components/shared/MapRecenter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import styles from "./MapRecenter.module.scss"
type MapRecenterProps = {
visibleMapMarkers: number
mapMarkers: ListingMapMarker[] | null
isLoading: boolean
}

const MapRecenter = (props: MapRecenterProps) => {
Expand All @@ -17,7 +18,8 @@ const MapRecenter = (props: MapRecenterProps) => {
if (
!map ||
props.visibleMapMarkers === undefined ||
props.visibleMapMarkers === props.mapMarkers.length
props.visibleMapMarkers === props.mapMarkers.length ||
props.isLoading
)
return null

Expand All @@ -38,6 +40,7 @@ const MapRecenter = (props: MapRecenterProps) => {
)
}}
size={"sm"}
variant={"primary-outlined"}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: Change the recenter button to primary outline so that it doesn't blend in with the larger clusters

>
{t("t.recenterMap")}
</Button>
Expand Down
19 changes: 11 additions & 8 deletions sites/public/src/pages/listings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React from "react"
import Head from "next/head"
import { APIProvider } from "@vis.gl/react-google-maps"
import { Heading } from "@bloom-housing/ui-seeds"
import { t } from "@bloom-housing/ui-components"
import { MetaTags } from "../components/shared/MetaTags"
Expand Down Expand Up @@ -44,14 +45,16 @@ export default function ListingsPage(props: ListingsProps) {
{t("nav.listings")}
</Heading>
{props.showAllMapPins === "TRUE" ? (
<ListingsSearchCombined
googleMapsApiKey={props.googleMapsApiKey}
googleMapsMapId={props.googleMapsMapId}
searchString={searchString}
bedrooms={props.bedrooms}
bathrooms={props.bathrooms}
counties={locations}
/>
<APIProvider apiKey={props.googleMapsApiKey}>
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: Move API provider up so that the map check is accessible when calling the listings API

<ListingsSearchCombined
googleMapsApiKey={props.googleMapsApiKey}
googleMapsMapId={props.googleMapsMapId}
searchString={searchString}
bedrooms={props.bedrooms}
bathrooms={props.bathrooms}
counties={locations}
/>
</APIProvider>
) : (
<ListingsSearchCombinedDeprecated
googleMapsApiKey={props.googleMapsApiKey}
Expand Down
Loading