Skip to content
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

Merged
merged 13 commits into from
Jul 1, 2021
Merged

Conversation

binh-dam-ibigroup
Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup commented Jun 30, 2021

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:

  1. Open the details of a field trip request
  2. Open the view dropup menu and click "Printable trip plan"

image

import { getTitle } from '../../util/state'

// Styles specific for rendering PrintFieldTripLayout.
const PrintLayout = styled.div`
Copy link
Member

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).

Copy link
Collaborator Author

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?

Copy link
Member

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.

Copy link
Collaborator Author

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()
Copy link
Member

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?

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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')
Copy link
Member

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?

Copy link
Collaborator Author

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)
Copy link
Member

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.

Copy link
Collaborator Author

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'
/>
Copy link
Member

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.

Copy link
Collaborator Author

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) {
Copy link
Member

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?

Copy link
Collaborator Author

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.

Copy link
Contributor

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.

Copy link
Member

@landonreed landonreed left a 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.

@landonreed landonreed removed their assignment Jun 30, 2021
@@ -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
Copy link
Contributor

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.

Copy link
Collaborator Author

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>
Copy link
Contributor

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?

Copy link
Collaborator Author

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.

Copy link
Contributor

@evansiroky evansiroky left a 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.

@evansiroky evansiroky removed their assignment Jun 30, 2021
@evansiroky evansiroky merged commit 1ed767a into dev Jul 1, 2021
@landonreed landonreed deleted the calltaker-print-group-itins branch July 1, 2021 18:25
@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2021

🎉 This PR is included in version 3.1.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.

3 participants