-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Calltaker: Print group itineraries #398
Conversation
import { getTitle } from '../../util/state' | ||
|
||
// Styles specific for rendering PrintFieldTripLayout. | ||
const PrintLayout = styled.div` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's put all of these styles in a styled.js
file (which is the approach we take elsewhere in this repo).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A separate styled-print.js?
or a variable in styled.js
for the print styles?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the styled-print.js
idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated in a266e27.
if (!prevProps.session && session) { | ||
// When session is set, load all field trips, | ||
// then load details for field trip per request id. | ||
await fetchFieldTrips() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trimet has thousands of field trips in its database. Is it possible to load a single field trip by request id?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably cheat around fetchFieldTrips
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestion, updated in 9e3a9ea.
*/ | ||
componentWillUnmount () { | ||
const root = document.getElementsByTagName('html')[0] | ||
root.removeAttribute('class') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class attr. stuff here and in componentDidMount
is duplicated in print-layout
. Should we just thrown that into a print util?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, done in 4339459.
request, | ||
requestId, | ||
session: state.callTaker.session, | ||
title: getTitle(state) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a better title might be the Field Trip: {schoolName} to {endLocation}
string, but this is non-blocking and we should just get TriMet input on this at a later point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, updated in e5ace26.
<Route | ||
component={PrintFieldTripLayout} | ||
path='/printFieldTrip' | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At some point we should probably figure out a solution to selectively include routes depending on config stuff. For now an issue is probably OK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noted in #399.
* LZW functions adapted from jsolait library (LGPL) | ||
* via http://stackoverflow.com/questions/294297/javascript-implementation-of-gzip | ||
*/ | ||
export function lzwEncode (s) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this used anywhere? I'm guessing it will be at some point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly in @evansiroky's PR. I'll leave it there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'll need this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. Just a few small items to address.
Co-authored-by: Landon Reed <landontreed@gmail.com>
…tripplanner/otp-react-redux into calltaker-print-group-itins
@@ -214,7 +223,8 @@ const mapStateToProps = (state, ownProps) => { | |||
currentQuery: state.otp.currentQuery, | |||
datastoreUrl: state.otp.config.datastoreUrl, | |||
dateFormat: getDateFormat(state.otp.config), | |||
request | |||
request, | |||
session: state.callTaker.session |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: session
could be sessionId
instead since session
isn't used except to get the sessionId
field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed in 6ef72b9.
<TripBody key={i}> | ||
<ItineraryContainer> | ||
<h3>{groupItin.passengers} passengers on following itinerary:</h3> | ||
<ItineraryBody> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got a little confused by this since we use itinerary-body
to render an itinerary. Maybe rename it ItineraryBodyContainer
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used the class name in the old client that's why. Renamed in 6ef72b9.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just two very non-blocking comments.
🎉 This PR is included in version 3.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This PR adds functionality for printing Field Trip itineraries that were previously planned and saved in otp-datastore.
Print functionality uses existing renders for itinerary narratives and trip detail summaries (some items printed by the native OTP client are thus missing).
To print a Field Trip itinerary: