-
Notifications
You must be signed in to change notification settings - Fork 1
MapFeaturesContext: Architecture Concept #320
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.
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
base: develop
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| import { createContext, useContext, useEffect, useState, type ReactNode } from 'react'; | ||
| import { Feature, FeatureCollection } from '@peripleo/peripleo'; | ||
| import { Typesense, useCachedHits } from '@performant-software/core-data'; | ||
| import { useSearchConfig } from '../SearchConfigContext'; | ||
|
|
||
| interface MapFeaturesContextType { | ||
| places: FeatureCollection; | ||
| updatePlace(feature: Feature): void; | ||
| } | ||
|
|
||
| const MapFeaturesContext = createContext<MapFeaturesContextType>(null); | ||
|
|
||
| interface Props { | ||
| children: ReactNode; | ||
| } | ||
|
|
||
| export const MapFeaturesContextProvider = ({ children }: Props) => { | ||
| const config = useSearchConfig(); | ||
| const hits = useCachedHits(); | ||
|
|
||
| // May have to be an empty feature collection instead! | ||
| const [places, setPlaces] = useState<FeatureCollection>(null); | ||
|
|
||
| /** | ||
| * Memo-izes the data to be displayed on the map as a feature collection. | ||
| */ | ||
| useEffect(() => { | ||
| const options = config.map.cluster_radius ? { type: 'Point' } : undefined; | ||
| const featureCollection = Typesense.toFeatureCollection(hits, config.map.geometry, options); | ||
|
|
||
| // TODO we could make this smarter! If a feature with the given ID already exists, | ||
| // we could keep it. (Assuming that the hits would never provide a higher-res version than | ||
| // what we already have.) | ||
| setPlaces(featureCollection); | ||
| }, [hits]); | ||
|
|
||
| const updatePlace = (feature: Feature) => { | ||
| const updated = places.features.map(f => f.id === feature.id ? feature : f); | ||
| setPlaces({ type: 'FeatureCollection', features: updated }); | ||
| } | ||
|
|
||
| return ( | ||
| <MapFeaturesContext.Provider | ||
| value={{ | ||
| places, | ||
| updatePlace | ||
| }} | ||
| > | ||
| { children } | ||
| </MapFeaturesContext.Provider> | ||
| ) | ||
| }; | ||
|
|
||
| export const useCachedPlaces = () => { | ||
| return useContext(MapFeaturesContext); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,8 @@ import { | |
| import _ from 'underscore'; | ||
| import PanelHistoryContext from '@apps/search/map/PanelHistoryContext'; | ||
| import { useSearchConfig } from '@apps/search/SearchConfigContext'; | ||
| import { preProcessFile } from 'typescript'; | ||
| import { useCachedPlaces } from '../MapFeaturesContext'; | ||
|
|
||
| interface Props { | ||
| className?: string; | ||
|
|
@@ -49,6 +51,7 @@ const BasePanel = (props: Props) => { | |
|
|
||
| const navigate = useNavigate(); | ||
| const config = useSearchConfig(); | ||
| const { updatePlace } = useCachedPlaces(); | ||
| const { t } = useContext(TranslationContext); | ||
| const { setSelected } = useSelection(); | ||
|
|
||
|
|
@@ -209,7 +212,7 @@ const { data: { people = [] } = {}, loading: peopleLoading } = useLoader(onLoadP | |
|
|
||
| /** | ||
| * Memo-izes the geometry. | ||
| */ | ||
| * | ||
| const geometry = useMemo(() => { | ||
| if (props.resolveGeometry && item) { | ||
| return props.resolveGeometry(item); | ||
|
|
@@ -220,6 +223,19 @@ const { data: { people = [] } = {}, loading: peopleLoading } = useLoader(onLoadP | |
| places.filter((place) => place.place_geometry) | ||
| ); | ||
| }, [item, places, props.resolveGeometry]); | ||
| */ | ||
|
|
||
| useEffect(() => { | ||
| if (props.resolveGeometry && item) { | ||
| const highRes = props.resolveGeometry(item); | ||
| updatePlace(highRes); | ||
| } else if (!_.isEmpty(places.filter((place) => place.place_geometry))) { | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TBH I didn't figure out what this branch does. Under what conditions will new places come in? I assume this code needs changing... instead of creating one FeatureCollection, we probably need to loop through each place and call
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For non-place records (people, events, etc), the geometry data is obtained from the related place records (fetched above). For this use case, the record could be linked to multiple geometries, so yes, I believe we'd need to loop through each and update. For place records, we use the |
||
| const place = CoreDataUtils.toFeatureCollection( | ||
| places.filter((place) => place.place_geometry) | ||
| ); | ||
| updatePlace(place); | ||
| } | ||
| }, [item, places, props.resolveGeometry]) | ||
|
|
||
| /** | ||
| * Memo-izes the related media items. | ||
|
|
@@ -450,7 +466,7 @@ const { data: { people = [] } = {}, loading: peopleLoading } = useLoader(onLoadP | |
| /> | ||
| )} | ||
| </RecordDetailPanel> | ||
| { geometry && ( | ||
| {/* geometry && ( | ||
| <LocationMarkers | ||
| animate | ||
| boundingBoxOptions={boundingBoxOptions} | ||
|
|
@@ -465,7 +481,7 @@ const { data: { people = [] } = {}, loading: peopleLoading } = useLoader(onLoadP | |
| fitBoundingBox={_.get(config.map, 'zoom_to_place', true)} | ||
| layerId='current' | ||
| /> | ||
| )} | ||
| ) */} | ||
| { manifestUrl && ( | ||
| <MediaGallery | ||
| manifestUrl={manifestUrl} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: no changes in this component, except for wrapping everything in
MapFeaturesContextProvider.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would the
MapSearchcomponent be a more appropriate place for this context than theMapLayout? Or was there a reason for specifically putting this inMapLayout?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also looks like there were come changes to CSS classes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The component that's rendering the selected geometry overlay is the
<BasePanel>which, if I saw correctly, is rendered in<SearchRoutes>– which is why<MapLayout>would be the lowest component in the hierarchy this would have to be, I think.Re CSS classes: I'll double check. But I think that's just an artefact of how git interprets the change of wrapping the whole markup.