enhance nearby task completion modal #2552
Conversation
| // componentDidUpdate(prevProps, prevState) { | ||
| // if ( | ||
| // this.state.nearbyTasks && | ||
| // !this.state.nearbyTasks.loading && | ||
| // this.props.taskId !== this.state.nearbyTasks.nearTaskId | ||
| // ) { | ||
| // this.updateNearbyTasks(this.props); | ||
| // } | ||
| // } | ||
|
|
There was a problem hiding this comment.
Nit: looks like this commented-out code can just be deleted
| const MapBounds = ({ taskMarkers, setMapBounds, loadByNearbyTasks, setLoadByNearbyTasks }) => { | ||
| const map = useMap(); | ||
| const prevMarkersLength = useRef(taskMarkers?.length || 0); | ||
| const initialBoundsSet = useRef(false); | ||
|
|
||
| // Only track map bounds changes | ||
| useEffect(() => { | ||
| const handleMoveEnd = () => { | ||
| const bounds = map.getBounds(); | ||
| setMapBounds(bounds); | ||
| }; | ||
|
|
||
| map.on("moveend", handleMoveEnd); | ||
|
|
||
| // Set initial bounds tracking | ||
| if (!initialBoundsSet.current) { | ||
| handleMoveEnd(); | ||
| initialBoundsSet.current = true; | ||
| } | ||
|
|
||
| return () => { | ||
| map.off("moveend", handleMoveEnd); | ||
| }; | ||
| }, [map, setMapBounds]); | ||
|
|
||
| // Separate effect for handling bounds fitting | ||
| useEffect(() => { | ||
| const currentLength = taskMarkers?.length || 0; | ||
|
|
||
| // Only fit bounds if explicitly loading by nearby tasks AND markers count changed | ||
| if (loadByNearbyTasks && currentLength > 0 && taskMarkers !== prevMarkersLength.current) { | ||
| const bounds = L.latLngBounds(taskMarkers.map((marker) => marker.position)); | ||
| map.fitBounds(bounds, { | ||
| padding: [40, 40], | ||
| maxZoom: 18, | ||
| }); | ||
| prevMarkersLength.current = currentLength; | ||
| setLoadByNearbyTasks(false); | ||
| } | ||
| }, [map, taskMarkers, loadByNearbyTasks]); | ||
|
|
||
| return null; | ||
| }; |
There was a problem hiding this comment.
It looks like this component doesn't actually return JSX, its only purpose is to set up some effects? If so you can write this as a custom hook. You'd name it useMapBounds() and I think setMapBounds might become a return value rather than an argument (similar to how useState returns a setter function that lets you change the state.
There was a problem hiding this comment.
Im honestly not sure how to do what you are describing. The reason i did it this way to ket access to the map context: const map = useMap();. I could make a new map component but that would be a much bigger change. Your approach could be better, i just don't know how to do it or how exactly it would work.
|
please resolve jake's comments and merge conflicts. happy to test and approve after |
CollinBeczak
force-pushed
the
improve-nearby-tasks-modal
branch
from
facdbac to
96cff53
Compare
Requires: maproulette/maproulette-backend#1170
resolves: #2161
partially resolves: #2546
Modal before:
Modal after
Before clicking select all task in view
after clicking select all task in view
