fix(SettingsSelectorPanel): Fix issue selecting a rental mode; Fix mi…

…ssing rental mode icons.

Fix issue when selecting a rental mode would select all similar rental modes (e.g. CAR_RENT). Fix
missing rental mode icons by falling back to getCompanyIcon.

packages/icons/src/trimet-mode-icons.js

@@ -1,21 +1,18 @@
 import React from "react";
 
-import * as Companies from "./companies";
 import * as TriMet from "./trimet";
 
 /**
- * Icons for all TriMet mode choices.
+ * Icons for all TriMet modes.
+ * Any hail and rental modes managed by one or multiple companies
+ * are optional but can be overriden here using the
+ * pattern <OTP_MODE>_<COMPANY_ID> (e.g. 'CAR_HAIL_UBER').
  */
 const triMetModeIcons = {
   BICYCLE: <TriMet.Bike />,
-  BICYCLE_RENT: <Companies.Biketown />,
   BUS: <TriMet.Bus />,
   CAR: <TriMet.Car />,
-  CAR_HAIL_LYFT: <Companies.Lyft />,
-  CAR_HAIL_UBER: <Companies.Uber />,
   CAR_PARK: <TriMet.Car />,
-  CAR_RENT_CAR2GO: <Companies.Car2go />,
-  CAR_RENT_REACHNOW: <Companies.Reachnow />,
   GONDOLA: <TriMet.AerialTram />,
   MICROMOBILITY: <TriMet.Micromobility />,
   MICROMOBILITY_RENT: <TriMet.Micromobility />,

packages/trip-form/src/SettingsSelectorPanel/index.js

@@ -17,7 +17,7 @@ import * as Styled from "../styled";
 import {
   getModeOptions,
   getTransitSubmodeOptions,
-  getCompanies,
+  getCompaniesForModeId,
   getCompaniesOptions,
   getBicycleOrMicromobilityModeOptions,
   isBike
@@ -78,27 +78,23 @@ export default class SettingsSelectorPanel extends Component {
         lastTransitModes = lastTransitModes.concat(allTransitModes);
       }
 
-      const nonTransitModes = newModes.length > 1 ? [newModes[1]] : ["WALK"]; // TODO: also accommodate WALK+DRIVE, WALK+e-scooter?? They already seem to work without WALK right now.
-      const defaultAccessModeCompany =
-        newModes.length > 2 ? [newModes[2]] : null; // To accommodate companies defined under accessModes.
+      const nonTransitModes = newModes.length > 1 ? [newModes[1]] : ["WALK"];
+      // TODO: also accommodate WALK+DRIVE, WALK+e-scooter?? They already seem to work without WALK right now.
+
+      const { defaultAccessModeCompany, companies } = getCompaniesForModeId(
+        id,
+        supportedCompanies
+      );
 
       // Add previously selected transit modes only if none were active.
       const finalModes = (activeTransitModes.length > 0
         ? activeTransitModes
         : lastTransitModes
       ).concat(nonTransitModes);
 
-      // If there are multiple (scooter | bikeshare) providers,
-      // then if one is specified by the mode button, select it,
-      // othewise select all providers.
-      // selectedCompanies is at least an empty array.
-      const selectedCompanies =
-        defaultAccessModeCompany ||
-        getCompanies(supportedCompanies, nonTransitModes).map(comp => comp.id);
-
       this.handleQueryParamChange({
         mode: finalModes.join(","),
-        companies: selectedCompanies.join(",")
+        companies: companies.join(",")
       });
 
       this.setState({
@@ -165,8 +161,15 @@ export default class SettingsSelectorPanel extends Component {
     } = this.props;
     const { defaultAccessModeCompany } = this.state;
     const selectedModes = this.getSelectedModes();
+    const selectedCompanies = this.getSelectedCompanies();
 
-    const modeOptions = getModeOptions(icons, supportedModes, selectedModes);
+    const modeOptions = getModeOptions(
+      icons,
+      supportedModes,
+      selectedModes,
+      selectedCompanies,
+      supportedCompanies
+    );
     const transitModes = getTransitSubmodeOptions(
       icons,
       supportedModes,
@@ -178,7 +181,7 @@ export default class SettingsSelectorPanel extends Component {
         defaultAccessModeCompany ? comp.id === defaultAccessModeCompany : true
       ),
       nonTransitModes,
-      this.getSelectedCompanies()
+      selectedCompanies
     );
     const bikeModes = getBicycleOrMicromobilityModeOptions(
       icons,

packages/trip-form/src/__mocks__/modes.js

@@ -41,6 +41,16 @@ const commonModes = {
       label: "Transit + Uber",
       company: "Uber" // Optional.
       // If not set and the user selects this, "companies" should default to Uber.
+    },
+    {
+      mode: "CAR_RENT",
+      label: "Transit + ReachNow",
+      company: "ReachNow"
+    },
+    {
+      mode: "CAR_RENT",
+      label: "Transit + Car2Go",
+      company: "Car2Go"
     }
   ],
   exclusiveModes: ["WALK", "BICYCLE", "MICROMOBILITY"],

packages/trip-form/src/settings-selector-panel.story.js

@@ -1,3 +1,4 @@
+import trimetModeIcons from "@opentripplanner/icons/lib/trimet-mode-icons"; // "../../icons/src/trimet-mode-icons";
 import React, { Component } from "react";
 import { action } from "@storybook/addon-actions";
 import { withInfo } from "@storybook/addon-info";
@@ -68,6 +69,7 @@ export default {
 export const settingsSelectorPanel = () => (
   <PanelWrapper>
     <SettingsSelectorPanel
+      icons={trimetModeIcons}
       supportedModes={commonModes}
       supportedCompanies={commonCompanies}
     />
@@ -87,6 +89,7 @@ export const settingsSelectorPanelWithCustomIcons = () => (
 export const settingsSelectorPanelUndefinedParams = () => (
   <PanelWrapper>
     <SettingsSelectorPanel
+      icons={trimetModeIcons}
       supportedModes={commonModesEmpty}
       supportedCompanies={undefined}
     />

packages/trip-form/src/util.js

@@ -47,6 +47,49 @@ export function getModeString(modeObj) {
   return modeObj.mode || modeObj;
 }
 
+/**
+ * Of the specified companies, returns those that operate the specified modes.
+ * @param {*} companies The supported companies per OTP configuration.
+ * @param {*} modes The desired modes for which to get the operating companies.
+ * @returns An array of companies that operate the specified modes (should not be undefined as companies is an array).
+ */
+function getCompanies(companies, modes) {
+  return companies
+    .filter(
+      comp => comp.modes.split(",").filter(m => modes.includes(m)).length > 0
+    )
+    .filter(comp => hasRental(comp.modes) || hasHail(comp.modes));
+}
+
+/**
+ * Returns an array containing the company ids, in upper case for MOD UI URLs, for the specified mode id.
+ * The mode id scheme is set and used by function getTransitCombinedModeOptions().
+ * @param {*} id The mode id to process.
+ * @param {*} supportedCompanies The list of supported companies (see structure in __mocks__/companies.js).
+ */
+export function getCompaniesForModeId(id, supportedCompanies) {
+  const newModes = id.split("+"); // Duplicate logic.
+  const nonTransitModes = newModes.length > 1 ? [newModes[1]] : ["WALK"]; // Duplicate logic.
+
+  // Accommodate companies defined under accessModes.
+  // Convert company ID to upper case for passing to MOD UI URL.
+  const defaultAccessModeCompany =
+    newModes.length > 2 ? [newModes[2].toUpperCase()] : null;
+
+  // If there are multiple (scooter | bikeshare | etc.) providers,
+  // then if one is specified by the mode button, select it,
+  // othewise select all providers.
+  // Convert company IDs to upper case for passing to MOD UI URL.
+  // selectedCompanies is at least an empty array.
+  const companies =
+    defaultAccessModeCompany ||
+    getCompanies(supportedCompanies, nonTransitModes).map(comp =>
+      comp.id.toUpperCase()
+    );
+
+  return { defaultAccessModeCompany, companies };
+}
+
 export function getTransitSubmodeOptions(icons, modes, selectedModes) {
   const { transitModes } = modes;
 
@@ -67,6 +110,11 @@ export function getTransitSubmodeOptions(icons, modes, selectedModes) {
   });
 }
 
+/**
+ * Returns big primary "Take Transit" choice.
+ * @param {*} icons The icons for rendering.
+ * @param {*} selectedModes An array of string that lists the modes selected for a trip query.
+ */
 function getPrimaryModeOption(icons, selectedModes) {
   return {
     id: "TRANSIT",
@@ -82,20 +130,60 @@ function getPrimaryModeOption(icons, selectedModes) {
   };
 }
 
-function getTransitCombinedModeOptions(icons, modes, selectedModes) {
+/**
+ * Returns the transit + access mode combinations.
+ * @param {*} icons The icon set to use.
+ * @param {*} modes The available modes to choose from.
+ * @param {*} selectedModes An array of string that lists the modes selected for a trip query.
+ * @param {*} selectedCompanies The companies to show as selected.
+ * @param {*} supportedCompanies The supported companies for certain modes.
+ */
+function getTransitCombinedModeOptions(
+  icons,
+  modes,
+  selectedModes,
+  selectedCompanies,
+  supportedCompanies
+) {
   const { accessModes } = modes;
   const modesHaveTransit = selectedModes.some(isTransit);
 
   return (
     accessModes &&
     accessModes.map(modeObj => {
       const modeStr = getModeString(modeObj);
+      const modeCompany = modeObj.company
+        ? modeObj.company.toUpperCase()
+        : null;
+
+      const id = `TRANSIT+${modeStr}${
+        modeObj.company ? `+${modeObj.company}` : ""
+      }`;
+
+      const { companies } = getCompaniesForModeId(id, supportedCompanies);
+      const modeMonopoly = companies[0];
+      const CompanyIcon = getCompanyIcon(modeCompany || modeMonopoly || "");
+
       return {
-        id: `TRANSIT+${modeStr}${modeObj.company ? `+${modeObj.company}` : ""}`,
-        selected: modesHaveTransit && selectedModes.includes(modeStr),
+        id,
+        selected:
+          modesHaveTransit &&
+          selectedModes.includes(modeStr) &&
+          (!selectedCompanies.length ||
+            !modeCompany ||
+            selectedCompanies.includes(modeCompany)),
         text: (
           <span>
-            {icons.TRANSIT}+{icons[modeStr]}
+            {icons.TRANSIT}+
+            {icons[modeStr] || icons[`${modeStr}_${modeCompany}`] || (
+              <CompanyIcon />
+            )}
+            {/* Access mode icons are processed in the order above, so that:
+             * - Any generic mode (e.g. BICYCLE_RENT) can be directly customized using `icons`,
+             * - Implementers can set icons for companies not in OTP-UI or override OTP-UI icons using `icons`,
+             *   using the scheme <OTP_MODE>_<COMPANY> (e.g. 'CAR_HAIL_UBER').
+             * - Icons for common companies (defined in the icons package) don't need to be specified in `icons`.
+             */}
           </span>
         ),
         title: modeObj.label
@@ -104,6 +192,12 @@ function getTransitCombinedModeOptions(icons, modes, selectedModes) {
   );
 }
 
+/**
+ * Returns the exclusive mode options.
+ * @param {*} icons The icon set to use.
+ * @param {*} modes The available modes to choose from.
+ * @param {*} selectedModes An array of string that lists the modes selected for a trip query.
+ */
 function getExclusiveModeOptions(icons, modes, selectedModes) {
   const { exclusiveModes } = modes;
 
@@ -127,30 +221,32 @@ function getExclusiveModeOptions(icons, modes, selectedModes) {
 /**
  * Generates the options (primary, secondary, tertiary) for the mode selector based on the modes read from config.yaml.
  * @param {*} modes The modes defined in config.yaml.
+ * @param {*} icons The icon set to use.
+ * @param {*} modes The available modes to choose from.
  * @param {*} selectedModes An array of string that lists the modes selected for a trip query.
+ * @param {*} selectedCompanies The companies to show as selected.
+ * @param {*} supportedCompanies The supported companies for certain modes.
  */
-export function getModeOptions(icons, modes, selectedModes) {
+export function getModeOptions(
+  icons,
+  modes,
+  selectedModes,
+  selectedCompanies,
+  supportedCompanies
+) {
   return {
     primary: getPrimaryModeOption(icons, selectedModes),
-    secondary: getTransitCombinedModeOptions(icons, modes, selectedModes),
+    secondary: getTransitCombinedModeOptions(
+      icons,
+      modes,
+      selectedModes,
+      selectedCompanies,
+      supportedCompanies
+    ),
     tertiary: getExclusiveModeOptions(icons, modes, selectedModes)
   };
 }
 
-/**
- * Of the specified companies, returns those that operate the specified modes.
- * @param {*} companies The supported companies per OTP configuration.
- * @param {*} modes The desired modes for which to get the operating companies.
- * @returns An array of companies that operate the specified modes (should not be undefined as companies is an array).
- */
-export function getCompanies(companies, modes) {
-  return companies
-    .filter(
-      comp => comp.modes.split(",").filter(m => modes.includes(m)).length > 0
-    )
-    .filter(comp => hasRental(comp.modes) || hasHail(comp.modes));
-}
-
 /**
  * Returns the UI options for the specified companies, modes, and selection.
  * @param {*} companies The supported companies per OTP configuration.