Correctly sort routes in route viewer by agency #201

evansiroky · 2020-07-17T01:07:28Z

This PR looks at the route id and parses out the agency ID and then attempts to match to a transit operator id in order to properly sort routes by multiple agencies. Also, the operator image is now shown in the route viewer from this fix.

~~To try this out, look at the route list with a deployment with multiple agencies.~~

This PR will now correctly sort routes in both the route viewer and stop viewer using the core-utils v3 logic and config if provided. As such, this updates all otp-ui packages to the latest version. Part of that update is correcting how the stops-overlay works so that it works with the zoom-based-overlay. One more thing that this PR does is now show a mode icon for each route in the route viewer and also an agency icon (if one is properly defined in the transit operators config).

landonreed · 2020-07-17T14:33:33Z

lib/components/viewers/route-viewer.js

@@ -93,14 +99,13 @@ class RouteViewer extends Component {
          {agencySortedRoutes
            .map(route => {
              // Find operator based on agency_id (extracted from OTP route ID).
-              // TODO: re-implement multi-agency logos for route viewer.
-              // const operator = operatorForRoute(transitOperators, route) || {}
+              const operator = operatorForRoute(transitOperators, route) || {}


If the operator is missing, the <img> tag is shown below, but it is a broken image. I think this default to empty object should be removed.

binh-dam-ibigroup

This is a great idea, although the condition for showing the image should be tweaked a little.

binh-dam-ibigroup · 2020-07-17T14:32:11Z

lib/components/viewers/route-viewer.js

-              // routes endpoint and causing this to only display transitOperators for
-              // the selected route.
-              // operator && <img src={operator.logo} style={{marginRight: '5px'}} height={25} />
+            {operator &&


On the configs that don't define a logo, this results in a "broken image link" placeholder to be shown, so the condition for showing <img> should rather be operator && operator.logo.

If no logo is found, should some other indicator for agency be used (e.g., name)?

landonreed · 2020-07-17T14:35:38Z

lib/components/viewers/route-viewer.js

@@ -62,7 +67,8 @@ class RouteViewer extends Component {
      : []
    const agencySortedRoutes = transitOperators.length > 0
      ? sortedRoutes.sort((a, b) => {
-        return operatorIndexForRoute(transitOperators, a) - operatorIndexForRoute(transitOperators, b)
+        return routeOperatorComparatorValue(transitOperators, a) -
+          routeOperatorComparatorValue(transitOperators, b)


I wonder if we should just be placing this agency-sorting logic within otp-ui? Perhaps not, but I think the question is worth asking.

This seems specific to the route-viewer, so I'd say if the route-viewer becomes an otp-ui component, then it'd make sense to put this in otp-ui core-utils.

landonreed

Just a few changes. But also, should this be moved to otp-ui?

E.g, the method getTransitOperatorFromConfig already exists:

https://github.com/opentripplanner/otp-ui/blob/382a189857bb6f1a94d37c66589d89193b07ec59/packages/itinerary-body/src/place-row.js#L15-L17

And it's invoked with agencyId (whereas you have changed it to route ID).

evansiroky · 2020-07-21T18:48:03Z

This PR is now dependent on opentripplanner/otp-ui#188.

evansiroky · 2020-08-05T06:17:11Z

This PR is now dependent on opentripplanner/otp-ui#189.

evansiroky · 2020-11-06T19:21:56Z

This PR is ready to go, except for being dependent on opentripplanner/otp-ui#221.

binh-dam-ibigroup

Looks good. There are some code and layout items that could be refined further, but I'm in favor to pass this as is.

binh-dam-ibigroup · 2020-11-09T23:12:08Z

lib/components/viewers/route-viewer.js

-                color
-              }}>
+          </RouteRowElement>
+          <ModeIconElement>


On configs that don't have operator logos, the current layout wastes a good bit of white space to the left of the mode icon, IMO.

This was to ensure that rotues with a background color on their label had sufficient space. I went ahead and changed the styling to reduce that spacing for labels with a white background color in 6cacf91.

binh-dam-ibigroup · 2020-11-09T23:21:58Z

lib/util/viewer.js

+ * Route model returns the mode as an integer type whereas the RouteShort model
+ * returns the mode string.
+ */
+export function getModeFromRoute (route) {


I feel this function should be in OTP-UI core-utils instead...

I thought that too, but nothing else in otp-ui needs this at the moment, so I just put it here to not create even more otp-ui PRs.

evansiroky · 2020-11-11T17:18:36Z

Ready for review!

landonreed · 2020-11-16T15:48:42Z

lib/components/app/batch-routing-panel.js

      mobile,
      showUserSettings
    } = this.props
    const actionText = mobile ? 'tap' : 'click'
    return (
-      <ViewerContainer>
+      <ViewerContainer ModeIcon={ModeIcon}>


Maybe out of scope for this PR, but is there a better way for us to pass around Leg/ModeIcon? Would it be useful to place them in the store on app startup (passed along with the OTP config object perhaps). I just find it a bit cumbersome to pass them to these connected components in this way.

See #274 which proposes to address this comment.

landonreed · 2020-11-16T16:36:32Z

lib/components/viewers/stop-viewer.js

+    const patternComparator = (patternA, patternB) => routeComparator(
+      patternA.route,
+      patternB.route
+    )


This doesn't seem to have a fallback sort if the routes are the same. Should we have it fall back on pattern headsign or something along those lines?

We can do that. I'd like to reuse a sorting helper that isn't exported from otp-ui. Can you please approve opentripplanner/otp-ui#226 to make this possible?

Fixed in d9410f9 and e110f9a

landonreed · 2020-11-16T20:01:56Z

lib/components/viewers/route-viewer.js

+
+const OperatorImg = styled.img`
+  height: 25px;
+  margin-right: 8px;


The operator image appears to be offset vertically slightly from the other row elements:

Fixed in 576730f

landonreed

Just a few minor comments. Oh, and it might be nice to update the example-config.yml to contain a valid operators config.

evansiroky · 2020-11-18T00:05:30Z

Ready for review again.

example-config.yml

landonreed

Looks good. Just a final question on how feeds with no agency id are handled.

BREAKING CHANGE: - Refactor various components and example.js file to more heavily use React's context API (see #274) - Update to use latest config schema compatible with opentripplanner/core-utils v3 (see #201)

github-actions · 2021-01-08T16:08:14Z

🎉 This PR is included in version 2.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

fix(route-viewer): correctly sort routes in route viewer by agency

3b5b1b5

evansiroky assigned landonreed and binh-dam-ibigroup Jul 17, 2020

landonreed reviewed Jul 17, 2020

View reviewed changes

binh-dam-ibigroup requested changes Jul 17, 2020

View reviewed changes

landonreed reviewed Jul 17, 2020

View reviewed changes

binh-dam-ibigroup assigned evansiroky and unassigned binh-dam-ibigroup Jul 17, 2020

landonreed removed their assignment Jul 17, 2020

landonreed requested changes Jul 17, 2020

View reviewed changes

evansiroky added 2 commits July 21, 2020 11:45

refactor(route-viewer): only show operator logo if available

97172c1

refactor: bump core-utils to 2.0.0-alpha.1 and use new route utils

2d976fb

evansiroky added the BLOCKED Blocked (waiting on another PR to be merged) label Jul 21, 2020

evansiroky added 4 commits November 5, 2020 17:08

Merge branch 'dev' into fix-multi-agency-route-viewer

75e250c

chore: update to latest otp-ui packages

89eb0c7

refactor: get stops-overlay working with zoom-based-markers

d0cfd7d

fix: use core-utils v3 to sort routes by agency

198e5e5

evansiroky added WIP Work in progress and removed BLOCKED Blocked (waiting on another PR to be merged) labels Nov 6, 2020

feat(route-viewer): add mode icon and use styled components

a03055f

evansiroky added BLOCKED Blocked (waiting on another PR to be merged) and removed WIP Work in progress labels Nov 6, 2020

test: fix snapshots

150767a

binh-dam-ibigroup approved these changes Nov 10, 2020

View reviewed changes

evansiroky added 2 commits November 11, 2020 09:15

build: update to @opentripplanner/core-utils v3.0.1

8d6504e

refactor: have better spacing between mode icons

6cacf91

evansiroky removed the BLOCKED Blocked (waiting on another PR to be merged) label Nov 11, 2020

evansiroky assigned landonreed and unassigned evansiroky Nov 11, 2020

landonreed reviewed Nov 16, 2020

View reviewed changes

landonreed requested changes Nov 16, 2020

View reviewed changes

landonreed assigned evansiroky and unassigned landonreed Nov 16, 2020

evansiroky mentioned this pull request Nov 17, 2020

Use dynamic contents via React context #274

Merged

evansiroky added 4 commits November 17, 2020 12:02

refactor(stop-viewer): sort by headsign after sorting routes

d9410f9

build: update to core-utils v3.0.3

e110f9a

test: update snapshot to reflect new pattern sorting

9831165

refactor(route-viewer): better align route row

576730f

evansiroky assigned landonreed and unassigned evansiroky Nov 18, 2020

docs: add entry in example-config showing transitOperators setup

f0d3e97

landonreed reviewed Nov 20, 2020

View reviewed changes

example-config.yml Outdated Show resolved Hide resolved

landonreed approved these changes Nov 20, 2020

View reviewed changes

landonreed assigned evansiroky and unassigned landonreed Nov 20, 2020

docs: improve config docs about transitOperators

98ef742

evansiroky merged commit da85709 into dev Nov 20, 2020

evansiroky mentioned this pull request Nov 20, 2020

Next release #277

Merged

github-actions bot added the released label Jan 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correctly sort routes in route viewer by agency #201

Correctly sort routes in route viewer by agency #201

evansiroky commented Jul 17, 2020 •

edited

Loading

landonreed Jul 17, 2020

binh-dam-ibigroup left a comment

binh-dam-ibigroup Jul 17, 2020

landonreed Jul 17, 2020

landonreed Jul 17, 2020 •

edited

Loading

evansiroky Jul 20, 2020

landonreed left a comment •

edited

Loading

evansiroky commented Jul 21, 2020

evansiroky commented Aug 5, 2020

evansiroky commented Nov 6, 2020

binh-dam-ibigroup left a comment

binh-dam-ibigroup Nov 9, 2020

evansiroky Nov 11, 2020

binh-dam-ibigroup Nov 9, 2020

evansiroky Nov 10, 2020

evansiroky commented Nov 11, 2020

landonreed Nov 16, 2020

evansiroky Nov 17, 2020

landonreed Nov 16, 2020 •

edited

Loading

evansiroky Nov 17, 2020

landonreed Nov 17, 2020

evansiroky Nov 18, 2020

landonreed Nov 16, 2020

evansiroky Nov 18, 2020

landonreed left a comment •

edited

Loading

evansiroky commented Nov 18, 2020

landonreed left a comment

github-actions bot commented Jan 8, 2021

Correctly sort routes in route viewer by agency #201

Correctly sort routes in route viewer by agency #201

Conversation

evansiroky commented Jul 17, 2020 • edited Loading

Choose a reason for hiding this comment

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

landonreed Jul 17, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

landonreed left a comment • edited Loading

Choose a reason for hiding this comment

evansiroky commented Jul 21, 2020

evansiroky commented Aug 5, 2020

evansiroky commented Nov 6, 2020

binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evansiroky commented Nov 11, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

landonreed Nov 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

landonreed left a comment • edited Loading

Choose a reason for hiding this comment

evansiroky commented Nov 18, 2020

landonreed left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 8, 2021

evansiroky commented Jul 17, 2020 •

edited

Loading

landonreed Jul 17, 2020 •

edited

Loading

landonreed left a comment •

edited

Loading

landonreed Nov 16, 2020 •

edited

Loading

landonreed left a comment •

edited

Loading