-
-
Notifications
You must be signed in to change notification settings - Fork 86
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
Group ETA on same routes with same ETA times (remote duplicate ones) #194
Group ETA on same routes with same ETA times (remote duplicate ones) #194
Conversation
…st match current available route after valid eta is returned
…ith same route no and same eta is returned
WalkthroughThe enhancements to Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/hooks/useStopEtas.tsx (2 hunks)
Additional comments not posted (5)
src/hooks/useStopEtas.tsx (5)
Line range hint
11-13
:
Initialization ofisTodayHoliday
looks good.The
isTodayHoliday
variable is correctly initialized using theuseContext
hook.
Line range hint
34-38
:
Usage ofisTodayHoliday
inrouteKeys
memoization looks good.The
isTodayHoliday
variable is correctly used to filter routes based on their availability.
101-165
: New logic for removing duplicate routes and selecting the best route looks good.The new logic efficiently removes duplicate routes and selects the best route using heuristic scoring.
Line range hint
168-174
:
Updated sorting logic looks good.The sorting logic correctly prioritizes the earliest arrival times for display.
195-195
: Usage ofisTodayHoliday
infetchData
callback looks good.The
isTodayHoliday
variable is correctly used in the fetchData callback to filter routes.
Description
This PR intends to solve issue #112. The issue of #112 will be more obvious when multiple bus stops are merged and more ETAs are shown in the list (due to PR #193).
Therefore, it is recommended that when merging PR #193, this PR (#194) is also merged, but these two PRs themselves have no dependencies of each other, just a recommendation.
It is also recommended to deploy to alpha site for testing first before merging to production.
Behaviour before and after the code changes are illustrated in below screenshot. Left side is current production build at hkbus.app, right side is my localhost version with latest code.
Criteria to decide which route is chosen to be displayed when merging
Routes are chosen based on score system. Generally speaking, the criteria is as follows:
At the end, the routes with best score wins and is chosen.
Summary by CodeRabbit
useStopEtas
functionality for better filtering of stop lists and handling of duplicate routes.