Skip to content

Conversation

@miles-grant-ibigroup
Copy link
Collaborator

@miles-grant-ibigroup miles-grant-ibigroup commented Aug 24, 2021

Expands the route viewer details to be a full fledged view that can fill the sidebar.

Piggyback PR of #435 for internationalization.

New components added in this PR are still missing internationalization, but these will be added once #435 is approved.

Should work with existing OTP instances, but has features designed to work explicitly with ibi-group/OpenTripPlanner#63. If not using the new version of OTP, route shapes will not appear (due to changed OTP endpoints) and realtime vehicles will not appear

@miles-grant-ibigroup miles-grant-ibigroup self-assigned this Aug 25, 2021
@miles-grant-ibigroup miles-grant-ibigroup marked this pull request as ready for review August 27, 2021 13:33
@miles-grant-ibigroup miles-grant-ibigroup changed the title Route Viewer Details Pane Route Viewer Details Pane and Realtime Vehicle Position Viewer Aug 27, 2021
@miles-grant-ibigroup
Copy link
Collaborator Author

Have implemented all changes and slimmed down RouteViewer.js extracting RouteRow out of it.

@binh-dam-ibigroup
Copy link
Collaborator

If not using the new version of OTP, route shapes will not appear (due to changed OTP endpoints) and realtime vehicles will not appear

Can this be a fallback instead?

@miles-grant-ibigroup
Copy link
Collaborator Author

Yes it can be! In 30f4a83

Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more recommendations for changes.

i18n/en-US.yml Outdated
Comment on lines 59 to 64
incoming_at: "approaching {stop}"
stopped_at: "doors open at {stop}"
in_transit_to: "next stop {stop}"

travellingAt: "travelling at {milesPerHour}"
relativeTime: "{seconds} ago"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason the first letter of the displayed text is not capitalized?

Copy link
Collaborator Author

@miles-grant-ibigroup miles-grant-ibigroup Sep 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personal preference, but I do think it looks cleaner and helps hide the fact that 3 rows of text are being shown with no graphics to balance it out

import React, { Component } from 'react'
import { connect } from 'react-redux'
import styled from 'styled-components'
import PropTypes from 'prop-types'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sort imports. (I recall we enabled prettier automatically on commits at one point?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in this repository yet I don't think. #447 adds a new eslint config that includes automatic sorting.

Until that's merged, manually sorted in 9a71395

@miles-grant-ibigroup
Copy link
Collaborator Author

Thanks for the comments Binh. All addressed

Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to go after fixing the typo and the import order.

@miles-grant-ibigroup
Copy link
Collaborator Author

Thanks Binh! All changes are addressed.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2021

🎉 This PR is included in version 3.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants