Skip to content

feat: see planned rout even when no actual ride was executed #1070

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

AvivAbachi
Copy link
Collaborator

@AvivAbachi AvivAbachi commented Mar 28, 2025

Description

Display the planned route before selecting a time.
Image

Changelog

  • SingleLineMapPage
    • Enhanced search functionality.
    • Improved event handling.
  • useSingleLineData
    • Use undefined values for startTime instead of '0'.
    • Separate the plannedRouteStops from filteredPositions to allow for independent loading.
    • Optimized and refactored code.
  • RouteSelector and FilterPositionsByStartTimeSelector
    • Replace '0' with undefined values.
  • MapContent
    • Fixed the duplication of keys in plannedRouteStops.
    • Optimized and refactored code.
  • useRecenterOnDataChange
    • The map also focuses when plannedRouteStops are available.
    • Optimized and refactored code.
  • Map.scss
    • Improved readability of .map-index-title in dark mode.
      image
    • Formatted the file.
  • singlelineTest.spec.ts
    • Add Test route appears after select route
    • Fix selector in tooltip appears after clicking on map point in single line map

@AvivAbachi AvivAbachi requested a review from NoamGaash as a code owner March 28, 2025 13:43
@AvivAbachi AvivAbachi linked an issue Mar 28, 2025 that may be closed by this pull request
Copy link
Contributor

github-actions bot commented Mar 28, 2025

@AvivAbachi AvivAbachi changed the title see planned rout even when no actual ride was executed feat: see planned rout even when no actual ride was executed Mar 28, 2025
Copy link
Member

@NoamGaash NoamGaash left a comment

Choose a reason for hiding this comment

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

Thanks for this progress! Seems like a great direction. Left a few intermediate comments :) 👏 🙏

@NoamGaash NoamGaash self-requested a review March 31, 2025 08:03
@NoamGaash
Copy link
Member

Thanks a lot for this work!
Some general comments-

  • some of my comments are opinionated. However, many of my opinions are wrong so let me know when you know better and feel free to skip those comments
  • this PR is big. It includes a lot of refactoring, which is good, but where it's possible, it's usually better to make smaller PRs for faster review cycles and flexibility :)

Thanks a lot! It's great to see your code. Few are the people who dive in that deep, it's great to see your ideas in action

@AvivAbachi AvivAbachi self-assigned this Apr 2, 2025
@AvivAbachi
Copy link
Collaborator Author

AvivAbachi commented Apr 2, 2025

Thanks for the feedback!
It was my mistake not working on a small PR, I will keep it in mind.
I found your comments very helpful.

  • On useRecenterOnDataChange, you have something to do with having arrays on the dependency list?

Is there anything else to change?

@NoamGaash
Copy link
Member

Thanks! It looks great
As a general guideline, I think it's preferable to choose primitive values for dependencies, especially for intrusive effects (like re-centering the map), but seems like you did a great job here. We can always improve things later on future PRs :)

@AvivAbachi AvivAbachi merged commit 01b778f into main Apr 4, 2025
17 of 18 checks passed
@AvivAbachi AvivAbachi deleted the 1050-see-planned-rout-even-when-no-actual-ride-was-executed branch April 4, 2025 16:12
@NoamGaash
Copy link
Member

@all-contributors could you please add @AvivAbachi as a code contributor?
Not sure if we already did

Copy link
Contributor

@NoamGaash

I've put up a pull request to add @AvivAbachi! 🎉

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.

See planned rout even when no actual ride was executed
2 participants