Skip to content

Conversation

@evansiroky
Copy link
Contributor

As a part of the map refactor we left out the elevation point marker display on the map. This PR adds it back in a new map component. This PR depends on opentripplanner/otp-ui#107 in order to work properly.

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

See comments about comments. Also, did you test this? I tried it out and the marker doesn't actually move when the user hovers over the position on the leg diagram
see elevation-marker-not-moving.mov.zip (click link to download, then unzip to play video). Not sure if the issue is in elevation point marker or leg diagram.

@binh-dam-ibigroup
Copy link
Collaborator

binh-dam-ibigroup commented Apr 27, 2020

@landonreed, @evansiroky is right about opentripplanner/otp-ui#107. To make this work, I am reverting the core-utils call to the old itinerary function with the corrected turlAlong syntax.
(EDIT: I will add a few TODO comments about that)

@codecov-io
Copy link

codecov-io commented Apr 27, 2020

Codecov Report

Merging #152 into dev will decrease coverage by 0.05%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #152      +/-   ##
==========================================
- Coverage   13.02%   12.96%   -0.06%     
==========================================
  Files         115      116       +1     
  Lines        4569     4588      +19     
  Branches     1321     1324       +3     
==========================================
  Hits          595      595              
- Misses       3373     3389      +16     
- Partials      601      604       +3     
Impacted Files Coverage Δ
lib/components/map/default-map.js 0.00% <0.00%> (ø)
lib/components/map/elevation-point-marker.js 0.00% <0.00%> (ø)
lib/util/itinerary.js 29.25% <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 1c91553...47d64a8. Read the comment docs.

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.

I'd prefer to make a release of otp-ui and update that dependency instead of adding a TODO.

@landonreed landonreed merged commit 1bb8c7e into dev Apr 28, 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 📦🚀

@binh-dam-ibigroup binh-dam-ibigroup deleted the elevation-point-marker branch December 18, 2023 22:12
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