This repository has been archived by the owner on Dec 7, 2021. It is now read-only.

refactor(app.js): Migrate to @opentripplanner icons #260

Merged

binh-dam-ibigroup merged 16 commits into with-otp-ui-refactor from with-custom-icons

Jul 1, 2020

Contributor

binh-dam-ibigroup commented May 11, 2020 •

edited

Loading

This PR migrates trimet-mod-otp to use the new @opentripplanner icon sets.
What changes (breaking):

The configure() function in config.js now defines:
- A new ModeIcon attribute that sets the icon set for the transit mode selection pane.
- A new LegIcon attribute that replaces applyCustomIconLogic and contains custom logic to determine icons based on travel leg.
The old customIcons are entirely removed.

Now depends on #265 .


          refactor(app.js): Migrate to @opentripplanner icons and update app ac…

a5c380b

…cordingly.

binh-dam-ibigroup marked this pull request as ready for review

May 11, 2020 22:35

binh-dam-ibigroup requested review from evansiroky and landonreed

May 11, 2020 22:35

landonreed reviewed

View reviewed changes

lib/config.js Outdated Show resolved Hide resolved

Contributor Author

binh-dam-ibigroup commented May 12, 2020

Also @landonreed or @evansiroky, I had to regenerate yarn.lock or I would get an error involving the moment package. Let me know if you are luckier regarding that.

landonreed reviewed

View reviewed changes

lib/config.js Outdated Show resolved Hide resolved

landonreed reviewed

View reviewed changes

lib/app.js Outdated Show resolved Hide resolved

landonreed reviewed

View reviewed changes

lib/app.js Outdated Show resolved Hide resolved

landonreed reviewed

View reviewed changes

lib/app.js Show resolved Hide resolved

landonreed suggested changes

View reviewed changes

Contributor

landonreed left a comment

This still splits the icon config stuff up quite a bit. Can we just consolidate icon config in config.js and leave the rest out?


          refactor icon config

69a2bc0

Contributor

landonreed commented May 12, 2020

CSS issues:

Destination icon is incorrect (should be double ring)

TriMet rose is too large in mode icon

Exclusive modes should be centered

radio button selections are not blue (here and in date/time)

Contributor Author

binh-dam-ibigroup commented May 12, 2020

That's a caveat of using the OTP-UI library. The default OTP-RR is ported over, so to avoid that you need to compile the desired style into OTP-RR before hand.

Contributor

landonreed commented May 12, 2020

I'm seeing a prop warning when I plan a trip: trip plan

Contributor

landonreed commented May 12, 2020

It looks like the arrow for the collapsible div for walking directions doesn't toggle up/down like it should:

landonreed and others added 4 commits

May 12, 2020 13:52


          refactor config.js, provide example

ec2391c


          fix comment

0af2049


          fix example config.js

0d09ee2


          docs(config.js): Fix comment typo.

Contributor Author

binh-dam-ibigroup commented May 15, 2020

@landonreed, I think we have now a pretty good visual parity between the two with my latest changes to opentripplanner/otp-react-redux#150.

binh-dam-ibigroup requested a review from landonreed

May 15, 2020 14:05

binh-dam-ibigroup assigned landonreed and evansiroky

landonreed and others added 3 commits

May 15, 2020 12:31


          add back LegIcon for print view

f5e0df8


          Merge branch 'with-custom-icons' into with-custom-icons-ltr

6dbeab8


          Merge pull request #261 from ibi-group/with-custom-icons-ltr

d64aeb2

refactor icon config

evansiroky reviewed

View reviewed changes

package.json Outdated Show resolved Hide resolved

evansiroky reviewed

View reviewed changes

lib/app.js Outdated Show resolved Hide resolved

evansiroky reviewed

View reviewed changes

lib/config-example.js Outdated Show resolved Hide resolved

binh-dam-ibigroup mentioned this pull request

Ui tweaks opentripplanner/otp-ui#140

Merged


          fix(icons): Remove customIcons in favor of custom ModeIcon/LegIcon in…

40ad443

… config.js.

BREAKING CHANGE: ustomIcons previously defined in config.yml must be moved into custom ModeIcon or
custom LegIcon components that you define in config.js.

binh-dam-ibigroup requested a review from evansiroky

May 22, 2020 15:13

binh-dam-ibigroup assigned evansiroky

landonreed approved these changes

View reviewed changes

Contributor

landonreed left a comment

Looks good! But let's be sure not to merge into master until the otp-rr release is performed and fully tested. In fact, I think it might actually be good to change the base branch from master to otp-ui-refactor (which should be branched from master).

landonreed removed their assignment

evansiroky reviewed

View reviewed changes

lib/config.js Outdated Show resolved Hide resolved

evansiroky suggested changes

View reviewed changes

Collaborator

evansiroky left a comment

See most recent comment (#260 (comment)). Also, there are merge conflicts.

evansiroky removed their assignment

binh-dam-ibigroup added the BLOCKED label

Contributor Author

binh-dam-ibigroup commented Jun 16, 2020

Blocked pending action on opentripplanner/otp-react-redux#171.


          fix(config.js, dependencies): Update dependencies and config.js for u…

f81f1c1

…se with OTP-RR 1.0 beta.

binh-dam-ibigroup removed the BLOCKED label

binh-dam-ibigroup added 2 commits

June 23, 2020 15:24


          Merge branch 'master' into with-custom-icons


          refactor(config.js): Use StandardBike, specify required members of co…

5ea9f8c

…nfigure().

binh-dam-ibigroup changed the base branch from master to with-otp-ui-refactor

June 23, 2020 19:54

binh-dam-ibigroup added 2 commits

June 23, 2020 16:41


          docs(config.js): Tweak comment

f1f3769


          Merge branch 'with-otp-ui-refactor' into with-custom-icons

d60cf9e

binh-dam-ibigroup requested a review from evansiroky

June 23, 2020 21:35

binh-dam-ibigroup assigned evansiroky and unassigned binh-dam-ibigroup

evansiroky suggested changes

View reviewed changes

Collaborator

evansiroky left a comment

Why is the branch changed to with-otp-ui-refactor? Can't we just use master for this?

evansiroky assigned binh-dam-ibigroup and unassigned evansiroky

binh-dam-ibigroup added the BLOCKED label

Contributor Author

binh-dam-ibigroup commented Jun 30, 2020

Blocking, awaiting #265.

binh-dam-ibigroup merged commit b86a587 into with-otp-ui-refactor

binh-dam-ibigroup deleted the with-custom-icons branch

July 1, 2020 16:13

binh-dam-ibigroup restored the with-custom-icons branch

July 1, 2020 16:13

binh-dam-ibigroup deleted the with-custom-icons branch

July 1, 2020 16:26

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

Labels

BLOCKED