Skip to content

Conversation

@evansiroky
Copy link
Contributor

@evansiroky evansiroky commented Mar 2, 2020

This PR replaces the location field and geocoding functionality using code sourced from otp-ui.

Also, this code fixes an annoying bug that would overlay the UserSettings component on the welcome screen when user data persistence was active.

@codecov-io
Copy link

codecov-io commented Mar 2, 2020

Codecov Report

❗ No coverage uploaded for pull request base (dev@c84de22). Click here to learn what that means.
The diff coverage is 3.22%.

Impacted file tree graph

@@          Coverage Diff           @@
##             dev     #137   +/-   ##
======================================
  Coverage       ?   10.13%           
======================================
  Files          ?      132           
  Lines          ?     5861           
  Branches       ?     1711           
======================================
  Hits           ?      594           
  Misses         ?     4469           
  Partials       ?      798
Impacted Files Coverage Δ
lib/util/ui.js 9.52% <ø> (ø)
lib/util/index.js 0% <ø> (ø)
lib/components/mobile/search-screen.js 0% <0%> (ø)
lib/components/mobile/welcome-screen.js 0% <0%> (ø)
lib/index.js 0% <0%> (ø)
lib/components/form/default-search-form.js 0% <0%> (ø)
lib/components/mobile/location-search.js 0% <0%> (ø)
lib/components/form/connected-location-field.js 0% <0%> (ø)
lib/actions/map.js 24.52% <16.66%> (ø)

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 c84de22...984e0c8. Read the comment docs.

@landonreed
Copy link
Member

@evansiroky, I believe the user settings overlay was a feature ;)

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.

A couple of UI tweaks are needed, I think:

  1. Hovering over an option has the item text underlined -- but I don't think the styling we had previously did this (see below)
  2. It seems like the onBlur behavior is not working. I.e., if I click in the location field and search for a place and then tab or click away, the dropdown still stays open.
    image

@binh-dam-ibigroup
Copy link
Collaborator

IMO the UI tweak should be addressed as a separate issue.

@evansiroky
Copy link
Contributor Author

See commit pushed to resolve styling issues. See opentripplanner/otp-ui#89 for fixes to blurring of location-field.

@binh-dam-ibigroup binh-dam-ibigroup removed their assignment Mar 19, 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.

Works great now, thanks for fixing the onBlur

@landonreed landonreed merged commit 40ae6e7 into dev Mar 19, 2020
@landonreed landonreed deleted the otp-ui-location-field branch March 19, 2020 14:23
@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