Merge pull request #5529 from HSLdevcom/AB#295

AB#295  fix route page map bugs

Author: sharhio

app/component/map/RoutePageMap.js

@@ -1,186 +1,166 @@
 /* eslint-disable no-underscore-dangle */
 import PropTypes from 'prop-types';
-import React from 'react';
+import React, { useEffect, useRef, useState } from 'react';
 import { createFragmentContainer, graphql } from 'react-relay';
-import { matchShape } from 'found';
 import connectToStores from 'fluxible-addons-react/connectToStores';
-import {
-  configShape,
-  mapLayerOptionsShape,
-  patternShape,
-  errorShape,
-} from '../../util/shapes';
+import { configShape, patternShape, errorShape } from '../../util/shapes';
 import MapWithTracking from './MapWithTracking';
 import RouteLine from './route/RouteLine';
 import VehicleMarkerContainer from './VehicleMarkerContainer';
 import { getStartTime } from '../../util/timeUtils';
 import withBreakpoint from '../../util/withBreakpoint';
 import BackButton from '../BackButton';
 import { isActiveDate } from '../../util/patternUtils';
-import { mapLayerShape } from '../../store/MapLayerStore';
 import { boundWithMinimumArea } from '../../util/geo-utils';
 import { getMapLayerOptions } from '../../util/mapLayerUtils';
 import CookieSettingsButton from '../CookieSettingsButton';
 
-class RoutePageMap extends React.Component {
-  constructor(props) {
-    super(props);
-    this.state = {
-      trackVehicle: !!this.props.match.params.tripId, // map follows vehicle
-    };
-  }
-
-  static propTypes = {
-    match: matchShape.isRequired,
-    pattern: patternShape.isRequired,
-    lat: PropTypes.number,
-    lon: PropTypes.number,
-    breakpoint: PropTypes.string.isRequired,
-    mapLayers: mapLayerShape.isRequired,
-    mapLayerOptions: mapLayerOptionsShape.isRequired,
-    trip: PropTypes.shape({ gtfsId: PropTypes.string }),
-    error: errorShape,
-  };
+function RoutePageMap(
+  { pattern, lat, lon, breakpoint, trip, error, ...rest },
+  { config },
+) {
+  const tripId = trip?.gtfsId;
+  const [trackVehicle, setTrackVehicle] = useState(!!tripId);
+  const tripIdRef = useRef();
+  const mwtRef = useRef();
+  const latRef = useRef();
+  const lonRef = useRef();
+  const bounds = useRef();
+  const code = useRef();
 
-  static defaultProps = {
-    trip: null,
-    lat: undefined,
-    lon: undefined,
-    error: undefined,
-  };
+  useEffect(() => {
+    // Throw error in client side if relay fails to fetch data
+    if (error && !pattern) {
+      throw error.message;
+    }
+  }, []);
 
-  static contextTypes = {
-    config: configShape.isRequired,
-  };
+  if (!pattern) {
+    return null;
+  }
 
-  // eslint-disable-next-line camelcase
-  UNSAFE_componentWillReceiveProps(nextProps) {
-    if (nextProps.match.params.tripId !== this.tripId) {
-      this.setState({
-        trackVehicle: !!nextProps.match.params.tripId,
-      });
+  useEffect(() => {
+    if (tripId !== tripIdRef.current) {
+      setTrackVehicle(!!tripId);
+      mwtRef.current?.disableMapTracking();
     }
-    if (
-      nextProps.match.params.tripId !== this.tripId ||
-      nextProps.pattern.code !== this.code
-    ) {
-      // clear tracking so that map can focus on desired things
-      this.mwtRef?.disableMapTracking();
+  }, [tripId]);
+
+  useEffect(() => {
+    if (pattern.code !== code.current) {
+      mwtRef.current?.disableMapTracking();
     }
-  }
+  }, [pattern.code]);
 
-  setMWTRef = ref => {
-    this.mwtRef = ref;
+  const setMWTRef = ref => {
+    mwtRef.current = ref;
   };
 
-  stopTracking = () => {
+  const stopTracking = () => {
     // filter events which occur when map moves by changed props
-    if (this.tripId === this.props.match.params.tripId) {
+    if (tripIdRef.current === tripId) {
       // user wants to navigate, allow it
-      this.setState({ trackVehicle: false });
+      setTrackVehicle(false);
     }
   };
 
-  componentDidMount() {
-    // Throw error in client side if relay fails to fetch data
-    if (this.props.error && !this.props.pattern) {
-      throw this.props.error.message;
-    }
-  }
-
-  render() {
-    const { pattern, lat, lon, match, breakpoint, mapLayers, mapLayerOptions } =
-      this.props;
-    if (!pattern) {
-      return false;
-    }
-    const { tripId } = match.params;
-    const mwtProps = {};
-    if (tripId && lat && lon) {
-      // already getting vehicle pos
-      if (this.state.trackVehicle) {
-        mwtProps.lat = lat;
-        mwtProps.lon = lon;
-        this.lat = lat;
-        this.lon = lon;
-        if (this.tripId !== tripId) {
-          setTimeout(() => {
-            this.tripId = tripId;
-          }, 500);
-        }
-      } else {
-        mwtProps.lat = this.lat;
-        mwtProps.lon = this.lon;
+  const mwtProps = {};
+  if (tripId && lat && lon) {
+    // already getting vehicle pos
+    if (trackVehicle) {
+      mwtProps.lat = lat;
+      mwtProps.lon = lon;
+      latRef.current = lat;
+      lonRef.current = lon;
+      if (tripIdRef.current !== tripId) {
+        setTimeout(() => {
+          tripIdRef.current = tripId;
+        }, 500);
       }
-      mwtProps.zoom = 16;
     } else {
-      if (this.code !== pattern.code || !this.bounds) {
-        let filteredPoints;
-        if (pattern.geometry) {
-          filteredPoints = pattern.geometry.filter(
-            point => point.lat !== null && point.lon !== null,
-          );
-        }
-        this.bounds = boundWithMinimumArea(
-          (filteredPoints || pattern.stops).map(p => [p.lat, p.lon]),
+      mwtProps.lat = latRef.current;
+      mwtProps.lon = lonRef.current;
+    }
+    mwtProps.zoom = 16;
+  } else {
+    if (code.current !== pattern.code || !bounds.current) {
+      let filteredPoints;
+      if (pattern.geometry) {
+        filteredPoints = pattern.geometry.filter(
+          point => point.lat !== null && point.lon !== null,
         );
-        this.code = pattern.code;
       }
-      if (this.tripId) {
-        // changed back to route view, force update
-        this.mwtRef?.forceRefresh();
-        this.tripId = undefined;
-      }
-      mwtProps.bounds = this.bounds;
-    }
-    const tripSelected =
-      this.props.trip && this.props.trip.gtfsId && isActiveDate(pattern);
-    let tripStart;
-    // BUG ??  tripStar prop is never set
-    const leafletObjs = [
-      <RouteLine
-        key="line"
-        pattern={pattern}
-        vehiclePosition={tripSelected ? { lat, lon } : null}
-      />,
-    ];
-    if (isActiveDate(pattern)) {
-      leafletObjs.push(
-        <VehicleMarkerContainer
-          key="vehicles"
-          direction={pattern.directionId}
-          pattern={pattern.code}
-          headsign={pattern.headsign}
-          topics={[pattern.route]}
-          tripStart={tripStart}
-        />,
+      bounds.current = boundWithMinimumArea(
+        (filteredPoints || pattern.stops).map(p => [p.lat, p.lon]),
       );
+      code.current = pattern.code;
     }
-
-    /* eslint-disable jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */
-    return (
-      <MapWithTracking
-        {...mwtProps}
-        className="full"
-        leafletObjs={leafletObjs}
-        mapLayers={mapLayers}
-        mapLayerOptions={mapLayerOptions}
-        onStartNavigation={this.stopTracking}
-        onMapTracking={this.stopTracking}
-        setMWTRef={this.setMWTRef}
-      >
-        {breakpoint !== 'large' && (
-          <BackButton
-            icon="icon_arrow-collapse--left"
-            iconClassName="arrow-icon"
-          />
-        )}
-        {this.context.config.useCookiesPrompt && <CookieSettingsButton />}
-      </MapWithTracking>
+    if (tripIdRef.current) {
+      // changed back to route view, force update
+      mwtRef.current?.forceRefresh();
+      tripIdRef.current = undefined;
+    }
+    mwtProps.bounds = bounds.current;
+  }
+  const tripSelected = lat && lon && trip?.gtfsId && isActiveDate(pattern);
+  const leafletObjs = [
+    <RouteLine
+      key="line"
+      pattern={pattern}
+      vehiclePosition={tripSelected ? { lat, lon } : null}
+    />,
+  ];
+  if (isActiveDate(pattern)) {
+    leafletObjs.push(
+      <VehicleMarkerContainer
+        key="vehicles"
+        direction={pattern.directionId}
+        pattern={pattern.code}
+        headsign={pattern.headsign}
+        topics={[pattern.route]}
+      />,
     );
   }
+  /* eslint-disable jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */
+  return (
+    <MapWithTracking
+      {...mwtProps}
+      className="full"
+      leafletObjs={leafletObjs}
+      onStartNavigation={stopTracking}
+      onMapTracking={stopTracking}
+      setMWTRef={setMWTRef}
+      {...rest}
+    >
+      {breakpoint !== 'large' && (
+        <BackButton
+          icon="icon_arrow-collapse--left"
+          iconClassName="arrow-icon"
+        />
+      )}
+      {config.useCookiesPrompt && <CookieSettingsButton />}
+    </MapWithTracking>
+  );
 }
 
+RoutePageMap.propTypes = {
+  pattern: patternShape.isRequired,
+  lat: PropTypes.number,
+  lon: PropTypes.number,
+  breakpoint: PropTypes.string.isRequired,
+  trip: PropTypes.shape({ gtfsId: PropTypes.string }),
+  error: errorShape,
+};
+
+RoutePageMap.defaultProps = {
+  trip: null,
+  lat: undefined,
+  lon: undefined,
+  error: undefined,
+};
+
+RoutePageMap.contextTypes = { config: configShape.isRequired };
+
 const RoutePageMapWithVehicles = connectToStores(
   withBreakpoint(RoutePageMap),
   ['RealTimeInformationStore', 'MapLayerStore'],
@@ -200,22 +180,14 @@ const RoutePageMapWithVehicles = connectToStores(
       const matchingVehicles = Object.keys(vehicles)
         .map(key => vehicles[key])
         .filter(
-          vehicle =>
-            vehicle.tripStartTime === undefined ||
-            vehicle.tripStartTime === tripStart,
-        )
-        .filter(
-          vehicle =>
-            vehicle.tripId === undefined || vehicle.tripId === trip.gtfsId,
-        )
-        .filter(
-          vehicle =>
-            vehicle.direction === undefined ||
-            vehicle.direction === Number(trip.directionId),
+          v =>
+            (v.tripStartTime === undefined || v.tripStartTime === tripStart) &&
+            (v.tripId === undefined || v.tripId === trip.gtfsId) &&
+            (v.direction === undefined ||
+              v.direction === Number(trip.directionId)),
         );
 
-      if (matchingVehicles.length !== 1) {
-        // no matching vehicles or cant distinguish between vehicles
+      if (!matchingVehicles.length) {
         return { mapLayers, mapLayerOptions };
       }
       const selectedVehicle = matchingVehicles[0];

app/component/map/VehicleMarkerContainer.js

@@ -79,7 +79,7 @@ function VehicleMarkerContainer(props, { config }) {
       return shouldShowVehicle(
         message,
         props.direction || desc?.direction,
-        props.tripStart || desc?.tripStart,
+        desc?.tripStart,
         props.pattern,
         ignoreHeadsign ? undefined : props.headsign,
         desc?.tripId,
@@ -134,7 +134,6 @@ function VehicleMarkerContainer(props, { config }) {
 }
 
 VehicleMarkerContainer.propTypes = {
-  tripStart: PropTypes.string,
   headsign: PropTypes.string,
   direction: PropTypes.number,
   vehicles: PropTypes.objectOf(
@@ -152,7 +151,6 @@ VehicleMarkerContainer.propTypes = {
 };
 
 VehicleMarkerContainer.defaultProps = {
-  tripStart: undefined,
   direction: undefined,
   useLargeIcon: true,
 };

app/routeRoutes.js

@@ -133,9 +133,9 @@ export default function routeRoutes(config) {
                   }
                 }
               `}
-              render={({ Component, props, error, match }) => {
+              render={({ Component, props, error }) => {
                 if (Component && (props || error)) {
-                  return <Component {...props} match={match} error={error} />;
+                  return <Component {...props} error={error} />;
                 }
                 return null;
               }}
@@ -154,16 +154,9 @@ export default function routeRoutes(config) {
                   }
                 }
               `}
-              render={({ Component, props, error, match }) => {
+              render={({ Component, props, error }) => {
                 if (Component && (props || error)) {
-                  return (
-                    <Component
-                      {...props}
-                      match={match}
-                      error={error}
-                      trip={null}
-                    />
-                  );
+                  return <Component {...props} error={error} trip={null} />;
                 }
                 return null;
               }}