Open
Conversation
Added a Rush Hour section with AM and PM time inputs and functionality to generate rush hour times.
Contributor
WalkthroughAdds a Rush Hour (Optional) UI to the timetable template for specifying AM/PM windows and intervals; client-side logic generates rush-hour departure times, merges/deduplicates them with manual times, updates the departure-times field and refreshes the preview. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
fleet/templates/timetable_add.html (1)
148-152: Step number mismatch.The comment correctly indicates "Step 4" but the displayed step number is still "3". With the new Rush Hour section being Step 3, this should be updated to Step 4 for proper sequencing.
Proposed fix
<!-- Step 4: Stop Offsets --> <div class="config-card"> <div class="card-header"> - <span class="step-number">3</span> + <span class="step-number">4</span> <h3>Journey Times</h3> </div>
🤖 Fix all issues with AI agents
In @fleet/templates/timetable_add.html:
- Around line 532-584: The file contains a duplicate applyRushHours() definition
prefixed by a malformed "}function" token (likely a copy-paste), so remove the
duplicate block (the second applyRushHours definition starting at the
"}function" token) leaving only the original applyRushHours() implementation;
ensure you delete the extraneous closing brace and the entire duplicate function
body and then run the page to verify generateStopTimesJSON() and any callers
still work as before.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
fleet/templates/timetable_add.html
🔇 Additional comments (2)
fleet/templates/timetable_add.html (2)
96-146: LGTM!The Rush Hour UI section follows the established pattern of other config cards and provides intuitive inputs for AM/PM rush periods.
480-531: Logic looks good.The rush hour time generation, merging, deduplication, and sorting logic is correctly implemented. However, see the next comment regarding the duplicate function definition that immediately follows.
fleet/templates/timetable_add.html
Outdated
Comment on lines
532
to
584
| }function applyRushHours() { | ||
| const amStart = document.getElementById("rush-am-start").value; | ||
| const amEnd = document.getElementById("rush-am-end").value; | ||
| const amInterval = parseInt(document.getElementById("rush-am-interval").value, 10); | ||
|
|
||
| const pmStart = document.getElementById("rush-pm-start").value; | ||
| const pmEnd = document.getElementById("rush-pm-end").value; | ||
| const pmInterval = parseInt(document.getElementById("rush-pm-interval").value, 10); | ||
|
|
||
| const timesInput = document.querySelector('.departure-times'); | ||
| let existingTimes = timesInput.value.split(',').map(t => t.trim()).filter(Boolean); | ||
|
|
||
| // Helper function to generate times for a rush period | ||
| function generateRushTimes(start, end, interval) { | ||
| if (!start || !end || !interval || interval <= 0) return []; | ||
|
|
||
| const times = []; | ||
| let [startHour, startMin] = start.split(":").map(Number); | ||
| const [endHour, endMin] = end.split(":").map(Number); | ||
|
|
||
| while (startHour < endHour || (startHour === endHour && startMin <= endMin)) { | ||
| times.push(`${String(startHour).padStart(2, "0")}:${String(startMin).padStart(2, "0")}`); | ||
| startMin += interval; | ||
| if (startMin >= 60) { | ||
| startHour += Math.floor(startMin / 60); | ||
| startMin = startMin % 60; | ||
| } | ||
| } | ||
|
|
||
| return times; | ||
| } | ||
|
|
||
| // Generate AM rush times | ||
| const amTimes = generateRushTimes(amStart, amEnd, amInterval); | ||
|
|
||
| // Generate PM rush times | ||
| const pmTimes = generateRushTimes(pmStart, pmEnd, pmInterval); | ||
|
|
||
| // Merge all times and remove duplicates | ||
| const allTimes = [...existingTimes, ...amTimes, ...pmTimes]; | ||
|
|
||
| // Sort times and remove duplicates | ||
| const uniqueTimes = [...new Set(allTimes)].sort((a, b) => { | ||
| const [aH, aM] = a.split(':').map(Number); | ||
| const [bH, bM] = b.split(':').map(Number); | ||
| return (aH * 60 + aM) - (bH * 60 + bM); | ||
| }); | ||
|
|
||
| timesInput.value = uniqueTimes.join(", "); | ||
| generateStopTimesJSON(); | ||
|
|
||
| alert(`Added ${amTimes.length} AM rush trips and ${pmTimes.length} PM rush trips.`); | ||
| } |
Contributor
There was a problem hiding this comment.
Remove duplicate applyRushHours() function.
This is an exact duplicate of the function defined on lines 480-531. The malformed }function on line 532 indicates this was likely a copy-paste error. The duplicate should be removed.
Proposed fix
alert(`Added ${amTimes.length} AM rush trips and ${pmTimes.length} PM rush trips.`);
-}function applyRushHours() {
- const amStart = document.getElementById("rush-am-start").value;
- const amEnd = document.getElementById("rush-am-end").value;
- const amInterval = parseInt(document.getElementById("rush-am-interval").value, 10);
-
- const pmStart = document.getElementById("rush-pm-start").value;
- const pmEnd = document.getElementById("rush-pm-end").value;
- const pmInterval = parseInt(document.getElementById("rush-pm-interval").value, 10);
-
- const timesInput = document.querySelector('.departure-times');
- let existingTimes = timesInput.value.split(',').map(t => t.trim()).filter(Boolean);
-
- // Helper function to generate times for a rush period
- function generateRushTimes(start, end, interval) {
- if (!start || !end || !interval || interval <= 0) return [];
-
- const times = [];
- let [startHour, startMin] = start.split(":").map(Number);
- const [endHour, endMin] = end.split(":").map(Number);
-
- while (startHour < endHour || (startHour === endHour && startMin <= endMin)) {
- times.push(`${String(startHour).padStart(2, "0")}:${String(startMin).padStart(2, "0")}`);
- startMin += interval;
- if (startMin >= 60) {
- startHour += Math.floor(startMin / 60);
- startMin = startMin % 60;
- }
- }
-
- return times;
- }
-
- // Generate AM rush times
- const amTimes = generateRushTimes(amStart, amEnd, amInterval);
-
- // Generate PM rush times
- const pmTimes = generateRushTimes(pmStart, pmEnd, pmInterval);
-
- // Merge all times and remove duplicates
- const allTimes = [...existingTimes, ...amTimes, ...pmTimes];
-
- // Sort times and remove duplicates
- const uniqueTimes = [...new Set(allTimes)].sort((a, b) => {
- const [aH, aM] = a.split(':').map(Number);
- const [bH, bM] = b.split(':').map(Number);
- return (aH * 60 + aM) - (bH * 60 + bM);
- });
-
- timesInput.value = uniqueTimes.join(", ");
- generateStopTimesJSON();
-
- alert(`Added ${amTimes.length} AM rush trips and ${pmTimes.length} PM rush trips.`);
}
+
function autoFillOffsets() {🤖 Prompt for AI Agents
In @fleet/templates/timetable_add.html around lines 532 - 584, The file contains
a duplicate applyRushHours() definition prefixed by a malformed "}function"
token (likely a copy-paste), so remove the duplicate block (the second
applyRushHours definition starting at the "}function" token) leaving only the
original applyRushHours() implementation; ensure you delete the extraneous
closing brace and the entire duplicate function body and then run the page to
verify generateStopTimesJSON() and any callers still work as before.
Removed the duplicate applyRushHours function that generated rush hour times based on user inputs.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @fleet/templates/timetable_add.html:
- Around line 148-151: The step number in the Journey Times/Stop Offsets card is
wrong: update the span with class "step-number" inside the card header (the <div
class="config-card"> / <div class="card-header"> block) to display "4" instead
of "3" so the steps read sequentially after Rush Hour (which is now step 3).
🧹 Nitpick comments (1)
fleet/templates/timetable_add.html (1)
480-532: LGTM! Rush hour logic is solid.The
applyRushHours()function correctly:
- Validates inputs and handles empty fields gracefully
- Generates rush hour times for both AM and PM periods
- Merges with existing times, deduplicates, and sorts chronologically
- Updates the UI and triggers preview regeneration
The user feedback via alert is a nice UX touch.
Consider extracting the time generation logic to reduce duplication.
The
generateRushTimeshelper (lines 493-510) contains logic nearly identical to the time generation inautoFillTimes()(lines 463-474). Consider extracting this into a shared utility function to follow the DRY principle.♻️ Suggested refactor
Extract a reusable function before
autoFillTimes:+function generateTimeRange(start, end, interval) { + if (!start || !end || !interval || interval <= 0) return []; + + const times = []; + let [startHour, startMin] = start.split(":").map(Number); + const [endHour, endMin] = end.split(":").map(Number); + + while (startHour < endHour || (startHour === endHour && startMin <= endMin)) { + times.push(`${String(startHour).padStart(2, "0")}:${String(startMin).padStart(2, "0")}`); + startMin += interval; + if (startMin >= 60) { + startHour += Math.floor(startMin / 60); + startMin = startMin % 60; + } + } + + return times; +} + function autoFillTimes() { const start = document.getElementById("autofill-start").value; const end = document.getElementById("autofill-end").value; const interval = parseInt(document.getElementById("autofill-interval").value, 10); const input = document.querySelector('.departure-times'); if (!start || !end || !interval || interval <= 0) { alert("Please enter valid start, end times and interval."); return; } - const times = []; - let [startHour, startMin] = start.split(":").map(Number); - const [endHour, endMin] = end.split(":").map(Number); - - while (startHour < endHour || (startHour === endHour && startMin <= endMin)) { - times.push(`${String(startHour).padStart(2, "0")}:${String(startMin).padStart(2, "0")}`); - startMin += interval; - if (startMin >= 60) { - startHour += Math.floor(startMin / 60); - startMin = startMin % 60; - } - } + const times = generateTimeRange(start, end, interval); input.value = times.join(", "); generateStopTimesJSON(); }Then update
applyRushHoursto use the shared function:function applyRushHours() { const amStart = document.getElementById("rush-am-start").value; const amEnd = document.getElementById("rush-am-end").value; const amInterval = parseInt(document.getElementById("rush-am-interval").value, 10); const pmStart = document.getElementById("rush-pm-start").value; const pmEnd = document.getElementById("rush-pm-end").value; const pmInterval = parseInt(document.getElementById("rush-pm-interval").value, 10); const timesInput = document.querySelector('.departure-times'); let existingTimes = timesInput.value.split(',').map(t => t.trim()).filter(Boolean); - // Helper function to generate times for a rush period - function generateRushTimes(start, end, interval) { - if (!start || !end || !interval || interval <= 0) return []; - - const times = []; - let [startHour, startMin] = start.split(":").map(Number); - const [endHour, endMin] = end.split(":").map(Number); - - while (startHour < endHour || (startHour === endHour && startMin <= endMin)) { - times.push(`${String(startHour).padStart(2, "0")}:${String(startMin).padStart(2, "0")}`); - startMin += interval; - if (startMin >= 60) { - startHour += Math.floor(startMin / 60); - startMin = startMin % 60; - } - } - - return times; - } - // Generate AM rush times - const amTimes = generateRushTimes(amStart, amEnd, amInterval); + const amTimes = generateTimeRange(amStart, amEnd, amInterval); // Generate PM rush times - const pmTimes = generateRushTimes(pmStart, pmEnd, pmInterval); + const pmTimes = generateTimeRange(pmStart, pmEnd, pmInterval); // Merge all times and remove duplicates const allTimes = [...existingTimes, ...amTimes, ...pmTimes]; // Sort times and remove duplicates const uniqueTimes = [...new Set(allTimes)].sort((a, b) => { const [aH, aM] = a.split(':').map(Number); const [bH, bM] = b.split(':').map(Number); return (aH * 60 + aM) - (bH * 60 + bM); }); timesInput.value = uniqueTimes.join(", "); generateStopTimesJSON(); alert(`Added ${amTimes.length} AM rush trips and ${pmTimes.length} PM rush trips.`); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
fleet/templates/timetable_add.html
🔇 Additional comments (1)
fleet/templates/timetable_add.html (1)
96-146: LGTM! Clean implementation of the Rush Hour UI.The optional Rush Hour section is well-structured and follows the existing form patterns consistently. The default 10-minute interval for rush hours is appropriate, and the empty time defaults properly support the optional nature of this feature.
Comment on lines
+148
to
151
| <!-- Step 4: Stop Offsets --> | ||
| <div class="config-card"> | ||
| <div class="card-header"> | ||
| <span class="step-number">3</span> |
Contributor
There was a problem hiding this comment.
Fix the step number inconsistency.
The Journey Times section still shows step number "3", but it should be "4" since Rush Hour is now step 3.
🔢 Proposed fix
- <!-- Step 4: Stop Offsets -->
<div class="config-card">
<div class="card-header">
- <span class="step-number">3</span>
+ <span class="step-number">4</span>
<h3>Journey Times</h3>
</div>🤖 Prompt for AI Agents
In @fleet/templates/timetable_add.html around lines 148 - 151, The step number
in the Journey Times/Stop Offsets card is wrong: update the span with class
"step-number" inside the card header (the <div class="config-card"> / <div
class="card-header"> block) to display "4" instead of "3" so the steps read
sequentially after Rush Hour (which is now step 3).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added a Rush Hour section with AM and PM time inputs and functionality to generate rush hour times.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.