Skip to content

Conversation

@binh-dam-ibigroup
Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup commented Mar 19, 2020

This PR brings components of OTP-UI trip-form package into OTP-react-redux.
The SettingsSelectorPanel and DateTimeSelector components have been replaced with their counterparts in OTP-UI. Form components that became unused have been removed.

The PR leaves only one thing out pending otp-ui PR 92. When you select an access mode (e.g. Transit + Bike), the title does not get bold (it was previously).

@binh-dam-ibigroup
Copy link
Collaborator Author

When did mastarm become so uptight on a11y requirements??

@codecov-io
Copy link

codecov-io commented Mar 19, 2020

Codecov Report

Merging #141 into dev will increase coverage by 0.60%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #141      +/-   ##
==========================================
+ Coverage   10.28%   10.88%   +0.60%     
==========================================
  Files         131      127       -4     
  Lines        5777     5458     -319     
  Branches     1694     1587     -107     
==========================================
  Hits          594      594              
+ Misses       4393     4130     -263     
+ Partials      790      734      -56     
Impacted Files Coverage Δ
...mponents/form/connected-settings-selector-panel.js 0.00% <0.00%> (ø)
lib/components/form/date-time-modal.js 0.00% <0.00%> (ø)
lib/components/form/styled.js 0.00% <0.00%> (ø)
lib/components/form/tabbed-form-panel.js 0.00% <0.00%> (ø)
lib/components/form/user-trip-settings.js 0.00% <0.00%> (ø)
lib/components/mobile/options-screen.js 0.00% <0.00%> (ø)
lib/index.js 0.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e05b0b...959b3a4. Read the comment docs.

@binh-dam-ibigroup binh-dam-ibigroup marked this pull request as ready for review March 20, 2020 16:06
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.

My only problem with all this is the non-alphabetic 3rd party package imports. @opentripplanner/core-utils, etc should be imported before react.

Edit: Oh also, the merge conflicts. I have a problem with those too.

@evansiroky evansiroky removed their assignment Mar 20, 2020
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.

LGTM, only comment is adding jsdoc to the UserTripSettings component. Please do that before merging.

@binh-dam-ibigroup binh-dam-ibigroup merged commit d18d3ed into dev Mar 30, 2020
@binh-dam-ibigroup binh-dam-ibigroup deleted the otp-ui-trip-settings branch March 30, 2020 14:31
@binh-dam-ibigroup binh-dam-ibigroup removed their assignment Mar 30, 2020
@landonreed
Copy link
Member

🎉 This PR is included in version 1.0.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@landonreed
Copy link
Member

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants