Skip to content

Conversation

@binh-dam-ibigroup
Copy link
Collaborator

@binh-dam-ibigroup binh-dam-ibigroup commented Apr 10, 2023

Description

This PR updates @opentripplanner/location-field to the latest version, notably:

  • Adjust event handlers according to new event definitions
  • Adjust styles
  • (Re-)enable mobile mode in mobile view

PR Checklist

  • Does the code follow accessibility standards (WCAG 2.1 AA Compliant)?
  • Are all languages supported (Internationalization/Localization)?
  • Are appropriate Typescript types implemented?

@miles-grant-ibigroup miles-grant-ibigroup added the BLOCKED Blocked (waiting on another PR to be merged) label Apr 12, 2023
@binh-dam-ibigroup binh-dam-ibigroup marked this pull request as ready for review May 1, 2023 20:05
@binh-dam-ibigroup binh-dam-ibigroup removed the BLOCKED Blocked (waiting on another PR to be merged) label May 1, 2023
Copy link
Collaborator

@miles-grant-ibigroup miles-grant-ibigroup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All is working well thanks for the changes!

Copy link
Contributor

@amy-corson-ibigroup amy-corson-ibigroup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small issue with the favorite places getting rendered in double parentheses, but otherwise everything looks great! Thank you so much for doing this

/**
* Create a LocationField location object from a persisted user location object.
*/
function makeLocationFieldLocation(favoriteLocation: Fields) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe makeLocationFieldObject? makeLocationFieldLocation does seem a little unwieldy, but if that feels more appropriate I'll defer to you.

lib/util/user.js Outdated
name: address || name,
// HACK: If a place name and address are provided, put the address in parentheses
// to mimic the existing LocationField behavior for "work" and "home".
name: address && name ? `${name} (${address})` : address || name,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unfortunately resulting in a double parenthesis on Home and Work!
image

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 catch! Fixed in e94f5a1.

@binh-dam-ibigroup binh-dam-ibigroup merged commit 31447f4 into dev May 3, 2023
@binh-dam-ibigroup binh-dam-ibigroup deleted the location-field-cleanup branch May 3, 2023 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants