DateTimeSelector from OTP-UI, Remove Profile Routing #139
Conversation
| departArrive={departArrive} | ||
| onQueryParamChange={setQueryParam} | ||
| time={time} | ||
| timeFormatLegacy={timeFormatLegacy} |
There was a problem hiding this comment.
Could you explain why we're using props called *Legacy? This kind of splitting of code paths within the components doesn't seem necessary -- we should not be considering the otp-react-redux project "legacy" software. I don't think I had a chance to review opentripplanner/otp-ui#32, otherwise, I wouldn't have supported the splitting of the handlers like this.
EDIT: OK, I'm seeing below that this is intended to support legacy browsers? This and the DateTimeSelector probably need more comments to document this. We may want to modify the prop names to something a little more clear. Without the comment in mapStateToProps, I would have had little idea what was going on here.
There was a problem hiding this comment.
I agree on adding more documentation to DateTimeSelector in OTP-ui. There is a tiny bit here.
There was a problem hiding this comment.
See my adjusted the comments that are relevant to OTP-RR.
There was a problem hiding this comment.
I added additional docs on the OTP-UI side, see opentripplanner/otp-ui#98
| // form components | ||
| DateTimeModal, | ||
| DateTimePreview, | ||
| DateTimeSelector, |
There was a problem hiding this comment.
Hmm, do we actually want to remove this component here? Or do we want to export the styled date time selector in its place? Just trying to think through how this library and its exports might continue to be used.
There was a problem hiding this comment.
Good question. @evansiroky what is your take?
There was a problem hiding this comment.
Seems like it should be in the index file. But, our only real use of otp-react-redux is via trimet-mod-otp so I'm not sure.
There was a problem hiding this comment.
We have been deleting the refactored components from index.js. See also here: https://github.com/opentripplanner/otp-react-redux/pull/147/files
…dateFormatLegacy and timeFormat
Codecov Report
@@ Coverage Diff @@
## dev #139 +/- ##
==========================================
+ Coverage 10.13% 10.28% +0.14%
==========================================
Files 132 131 -1
Lines 5861 5777 -84
Branches 1711 1694 -17
==========================================
Hits 594 594
+ Misses 4469 4393 -76
+ Partials 798 790 -8
Continue to review full report at Codecov.
|
|
🎉 This PR is included in version 1.0.0-beta.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Checklist
devbefore they can be merged tomaster)Description
This PR replaces the DateTimeSelector component with the one from OTP-UI. For the DateTimeSelector component, the bootstrap and OTP-rr CSS attributes are reproduced inside a styled-component implementation.
The PR also removes code dealing with profile routing, most of which was previously commented out.