refactor: create MapContent component #743
Conversation
menachem95
changed the title
| @@ -147,7 +68,7 @@ export function MapIndex({ | |||
| ) | |||
| } | |||
|
|
|||
| function RecenterOnDataChange({ positions, plannedRouteStops }: MapProps) { | |||
| export function RecenterOnDataChange({ positions, plannedRouteStops }: MapProps) { | |||
There was a problem hiding this comment.
can it be moved to the other file?
|
Fantastic! |
|
@menachem95 do you need any help? I just saw Noam's great suggestion to improve UX regarding the hook you created - do you need clarifications about it? Btw, I wanna mention that on this Tuesday 8PM there will be a Q&A session so you can ask there if you prefer Maakaf discord |
|
Hi @menachem95 , could you please try |
|
Yeah, I think I'm going to need some guidance with that. |
|
feel free to consult everyone in the discord and slack groups, and please let us know how can we make the contribution experience smoother |
|
allright, seems there's a little circular dependency problem - two files are importing each other
|
| // const positionsSum = positions.reduce( | ||
| // (acc, { loc }) => [acc[0] + loc[0], acc[1] + loc[1]], | ||
| // [0, 0], | ||
| // ) | ||
| // const mean: LatLngTuple = [ | ||
| // positionsSum[0] / positions.length || position.loc[0], | ||
| // positionsSum[1] / positions.length || position.loc[1], | ||
| // ] |
There was a problem hiding this comment.
great! let's just remove the comments
| export interface MapProps { | ||
| positions: Point[] | ||
| plannedRouteStops: BusStop[] | ||
| } |
There was a problem hiding this comment.
exporting this to a different file (map-types.ts?) can solve the circular dependency
| import { useEffect } from 'react' | ||
| import { LatLngTuple } from 'leaflet' | ||
| import { useMap } from 'react-leaflet' | ||
| import { position } from './MapWithLocationsAndPath' |
There was a problem hiding this comment.
it creates a circular dependency
maybe you can avoid using it?
import { useEffect } from 'react'
import { LatLngTuple } from 'leaflet'
import { useMap } from 'react-leaflet'
import { position } from './MapWithLocationsAndPath'
import { MapProps } from './map-types'
export function useRecenterOnDataChange({ positions, plannedRouteStops }: MapProps) {
const map = useMap()
const top = Math.min(...positions.map((pos) => pos.loc[0]))
const left = Math.min(...positions.map((pos) => pos.loc[1]))
const bottom = Math.max(...positions.map((pos) => pos.loc[0]))
const right = Math.max(...positions.map((pos) => pos.loc[1]))
const center = [(top + bottom) / 2, (left + right) / 2] as LatLngTuple
const zoom = center[0]
? map.getBoundsZoom([
[top, left],
[bottom, right],
])
: undefined
useEffect(() => {
if (center[0] && center[1] && zoom) map.setView(center, zoom, { animate: true })
}, [...center])
} |
| import '../../Map.scss' | ||
|
|
||
| const position: Point = { | ||
| export const position: Point = { |
There was a problem hiding this comment.
this cause a circular dependency error -
✖ Found 1 circular dependency!
1) src/pages/components/map-related/MapContent.tsx > src/pages/components/map-related/useRecenterOnDataChange.ts > src/pages/components/map-related/MapWithLocationsAndPath.tsx
There was a problem hiding this comment.
it seems great!
but you do have this error to handle:
#14 10.47 src/pages/lineProfile/LineProfile.tsx(115,8): error TS2741: Property 'position' is missing in type '{ positions: Point[]; plannedRouteStops: BusStop[]; }' but required in type 'MapProps'.
#14 10.47 src/pages/singleLineMap/index.tsx(136,8): error TS2741: Property 'position' is missing in type '{ positions: Point[]; plannedRouteStops: BusStop[]; }' but required in type 'MapProps'.
also, I've added some optional comments
|
Good job, thanks! |
|
I've put up a pull request to add @menachem95! 🎉 |
Description
refactor: I built the component
screenshots