transitRouting: Add button to sort alternatives by trip length#1772
transitRouting: Add button to sort alternatives by trip length#1772Ayelander wants to merge 1 commit into
Conversation
WalkthroughAdded duration-based sorting to RoutingResultComponent: introduced Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx (1)
101-110:⚠️ Potential issue | 🟡 MinorThe
useEffectfires the asyncshowCurrentAlternativewithout cleanup.If the component re-renders rapidly (e.g., fast arrow-key presses), multiple in-flight async calls can resolve out of order, updating the map with stale data. Consider using an
ignoreflag pattern orAbortControllerto discard stale results.Sketch of stale-closure guard
useEffect(() => { + let cancelled = false; if (error) { return; } const shouldFitBounds = isInitialRenderRef.current; - showCurrentAlternative(result, alternativeIndex, shouldFitBounds); + showCurrentAlternative(result, alternativeIndex, shouldFitBounds).then(() => { + // If a newer effect has fired, skip any late side-effects here + if (cancelled) return; + }); isInitialRenderRef.current = false; + return () => { cancelled = true; }; }, [result, alternativeIndex, error]);
🤖 Fix all issues with AI agents
In
`@packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx`:
- Around line 167-175: The sort Button rendered when alternativesCount > 1 lacks
an accessible name (it uses label=""), so update the RoutingResultComponent to
provide an internationalized accessible label/tooltip by importing
useTranslation and passing t('routing.sort_by_duration') (or similar new i18n
key) into the Button via aria-label and/or title props; ensure you keep the
existing props (icon, color based on sortedByDuration, onClick ->
onSortButtonClick) and add the new t() key to your i18n files.
- Around line 83-99: displayIndex can become out-of-bounds when a new result
with fewer alternatives arrives; add a useEffect that watches result (or
alternativesCount and sortedByDuration) and resets/clamps displayIndex via
setDisplayIndex to a valid index (e.g., 0 or Math.min(current,
alternativesCount-1)) so alternativeIndex computed from sortedIndices (built by
buildSortedIndices) is never undefined before calling showCurrentAlternative or
getPath; if alternativesCount is 0 set displayIndex to 0 and ensure
sortedIndices-based mapping is handled safely.
| const [displayIndex, setDisplayIndex] = useState(0); | ||
| const [sortedByDuration, setSortedByDuration] = useState(false); | ||
| // Track if this is the first render to fit bounds only on initial display | ||
| const isInitialRenderRef = useRef(true); | ||
|
|
||
| const result = props.result; | ||
| const error = result.getError(); | ||
| const alternativesCount = result.getAlternativesCount(); | ||
|
|
||
| // Compute sorted indices when sorting is active | ||
| const sortedIndices = React.useMemo( | ||
| () => (sortedByDuration ? buildSortedIndices(result) : null), | ||
| [result, sortedByDuration] | ||
| ); | ||
|
|
||
| // Map display position to the actual alternative index | ||
| const alternativeIndex = sortedIndices ? sortedIndices[displayIndex] : displayIndex; |
There was a problem hiding this comment.
displayIndex is not reset when result changes — potential out-of-bounds access.
If a new routing result has fewer alternatives than the previous one, displayIndex may exceed the new alternativesCount, causing sortedIndices[displayIndex] (or the direct index) to be undefined. This would pass undefined as alternativeIndex to showCurrentAlternative and getPath, likely breaking the UI silently.
Consider adding an effect to reset the index when the result changes:
Proposed fix
const [displayIndex, setDisplayIndex] = useState(0);
const [sortedByDuration, setSortedByDuration] = useState(false);
+
+ // Reset display index when the routing result changes
+ useEffect(() => {
+ setDisplayIndex(0);
+ }, [result]);🤖 Prompt for AI Agents
In
`@packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx`
around lines 83 - 99, displayIndex can become out-of-bounds when a new result
with fewer alternatives arrives; add a useEffect that watches result (or
alternativesCount and sortedByDuration) and resets/clamps displayIndex via
setDisplayIndex to a valid index (e.g., 0 or Math.min(current,
alternativesCount-1)) so alternativeIndex computed from sortedIndices (built by
buildSortedIndices) is never undefined before calling showCurrentAlternative or
getPath; if alternativesCount is 0 set displayIndex to 0 and ensure
sortedIndices-based mapping is handled safely.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx">
<violation number="1" location="packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx:99">
P1: `displayIndex` is not reset when `result` changes, which can cause out-of-bounds access. If a new routing result has fewer alternatives than the previous one, `displayIndex` may exceed the new `alternativesCount`, resulting in `undefined` being passed as `alternativeIndex`. Add a `useEffect` to reset `displayIndex` to 0 when `result` changes.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| ); | ||
|
|
||
| // Map display position to the actual alternative index | ||
| const alternativeIndex = sortedIndices ? sortedIndices[displayIndex] : displayIndex; |
There was a problem hiding this comment.
P1: displayIndex is not reset when result changes, which can cause out-of-bounds access. If a new routing result has fewer alternatives than the previous one, displayIndex may exceed the new alternativesCount, resulting in undefined being passed as alternativeIndex. Add a useEffect to reset displayIndex to 0 when result changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx, line 99:
<comment>`displayIndex` is not reset when `result` changes, which can cause out-of-bounds access. If a new routing result has fewer alternatives than the previous one, `displayIndex` may exceed the new `alternativesCount`, resulting in `undefined` being passed as `alternativeIndex`. Add a `useEffect` to reset `displayIndex` to 0 when `result` changes.</comment>
<file context>
@@ -60,12 +80,23 @@ const showCurrentAlternative = async (
+ );
+
+ // Map display position to the actual alternative index
+ const alternativeIndex = sortedIndices ? sortedIndices[displayIndex] : displayIndex;
// Use effect to show the current alternative and fit bounds on initial render
</file context>
|
I have not checked the code, but is the design exensible so that we can have other way to sort? Number of transfer could be an interesting one to have. Another one I can think of is longest walk. Don't need them now, but if we can have space to easily add later. |
tahini
left a comment
tahini
left a comment
There was a problem hiding this comment.
L'approche est simple, c'est purement UI et interne au composant par contre, on pourrait éventuellement avoir un RoutingResultSorter qui serait hors du UI et qui pourrait prendre une mini-fonction en paramètre pour dire sur quel(s) champs trier. Peut-être ajouter un FIXME sur ces fonctions pour considérer déplacer ça dans transition-common au besoin.
Aussi, j'ai l'impression ici que l'ordre de tri va toujours revenir à l'état initial à chaque mise à jour de résultat. Je crois qu'il faudrait mettre le choix dans les préférences (pour qu'il survive dans toutes les sessions de l'utilisateur) ou du moins dans le composant qui affiche les résultats si on veut juste cette option pour les calculs successifs d'une séance. Si on l'ajoute dans les préférences, celles-ci sont plutôt mal documentées, mais je mettrais dans transit.routing.sortAlternativesBy avec, comme valeur pour l'instant soit none et travelTime
| icon={faArrowDownShortWide} | ||
| color={sortedByDuration ? 'green' : 'grey'} | ||
| iconClass="_icon-alone" | ||
| label="" |
There was a problem hiding this comment.
ça prendrait un label ou, du moins, un tooltip pour ce bouton.
|
@Ayelander y a-t-il des mises à jour sur cette PR? Le formatage ne passe pas et il faudrait aussi ajouter le label pour le bouton. Les autres commentaires sont pour un futur éventuel (préférences et sortAlternativesBy) et n'ont pas à être implémentées ici. |
Remi4001
force-pushed
the
main
branch
2 times, most recently
from
6f5aecd to
7ed461a
Compare
J'ai ajouté un title pour le tooltip, plus les commentaires FIXME pour plus tard. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx (1)
86-103:⚠️ Potential issue | 🟠 MajorClamp
displayIndexwhenresultchanges.A shorter replacement result can still leave
sortedIndices[displayIndex]undefined, which then propagates intogetPathandshowCurrentAlternative.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx` around lines 86 - 103, When the incoming result changes (or when sortedByDuration/sortedIndices/alternativesCount change) displayIndex can point past the available alternatives; add a useEffect watching result, alternativesCount, sortedByDuration, and sortedIndices that clamps displayIndex via setDisplayIndex to a valid range (>= 0 and < currentCount), using the length of sortedIndices when sorting is active and alternativesCount otherwise so that alternativeIndex (computed from sortedIndices and displayIndex) is never undefined before calling getPath or showCurrentAlternative.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx`:
- Around line 170-178: The i18n key passed to the Button title uses colons for
nested keys; update the lookup in RoutingResultComponent where the Button (icon
faArrowDownShortWide, onClick onSortButtonClick) sets title using props.t to use
dots for nested keys after the namespace separator (e.g. change the string
format from 'transit:transitPath:sort:TravelTime' to
'transit:transitPath.sort.TravelTime') so i18next resolves the nested key
correctly.
---
Duplicate comments:
In
`@packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx`:
- Around line 86-103: When the incoming result changes (or when
sortedByDuration/sortedIndices/alternativesCount change) displayIndex can point
past the available alternatives; add a useEffect watching result,
alternativesCount, sortedByDuration, and sortedIndices that clamps displayIndex
via setDisplayIndex to a valid range (>= 0 and < currentCount), using the length
of sortedIndices when sorting is active and alternativesCount otherwise so that
alternativeIndex (computed from sortedIndices and displayIndex) is never
undefined before calling getPath or showCurrentAlternative.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1f4c23a8-c03d-4dec-a3db-0b8546a66d79
📒 Files selected for processing (3)
locales/en/transit.jsonlocales/fr/transit.jsonpackages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.json
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
Translation files in
locales/directory should organize strings by namespace (auth.json, transit.json, main.json, etc.) with support for both English (en/) and French (fr/) languages
Files:
locales/en/transit.jsonlocales/fr/transit.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
**/*.{ts,tsx}: Use UUIDs as primary keys for most entities in the database
Use the Status/Result pattern from chaire-lib-common for service functions and handlers
Indent using 4 spaces
Do not use trailing spaces, even in comments or documentation; replace lines with only spaces with blank lines
Use parentheses for arrow function parameters
Document non-trivial functions and attributes using JSDoc or inline comments when short
Use Geographic data in GeoJSON format with coordinates as [longitude, latitude]
Use i18next and thet()function for all user-facing strings to support internationalization
Files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
packages/transition-frontend/src/components/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
React components should use Redux for state management, i18next for translations, and SCSS for styling
Files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
🧠 Learnings (7)
📚 Learning: 2026-02-03T20:40:36.982Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2026-02-03T20:40:36.982Z
Learning: Applies to **/*.json : Translation files in `locales/` directory should organize strings by namespace (auth.json, transit.json, main.json, etc.) with support for both English (en/) and French (fr/) languages
Applied to files:
locales/en/transit.jsonlocales/fr/transit.json
📚 Learning: 2026-02-03T20:40:36.982Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2026-02-03T20:40:36.982Z
Learning: Applies to packages/transition-frontend/src/components/routers/*.tsx : Application routing should be defined in `TransitionRouter.tsx` using React Router with feature-based route organization
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📚 Learning: 2026-02-03T20:40:36.982Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2026-02-03T20:40:36.982Z
Learning: Applies to packages/transition-frontend/src/components/**/*.tsx : React components should use Redux for state management, i18next for translations, and SCSS for styling
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📚 Learning: 2026-02-03T20:40:36.982Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2026-02-03T20:40:36.982Z
Learning: Applies to **/*.{ts,tsx} : Use the Status/Result pattern from chaire-lib-common for service functions and handlers
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📚 Learning: 2025-10-20T18:31:07.612Z
Learnt from: greenscientist
Repo: chairemobilite/transition PR: 1516
File: packages/chaire-lib-backend/src/utils/trRouting/TrRoutingServiceBackend.ts:29-41
Timestamp: 2025-10-20T18:31:07.612Z
Learning: In TrRoutingServiceBackend (packages/chaire-lib-backend/src/utils/trRouting/TrRoutingServiceBackend.ts), the fetch-retry package was not working as expected, so custom retry logic was implemented instead.
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📚 Learning: 2025-09-25T16:09:00.577Z
Learnt from: greenscientist
Repo: chairemobilite/transition PR: 1494
File: packages/transition-backend/src/services/transitRouting/TrAccessibilityMapBatch.ts:42-45
Timestamp: 2025-09-25T16:09:00.577Z
Learning: In packages/transition-backend/src/services/transitRouting/TrAccessibilityMapBatch.ts, greenscientist deferred adding input file validation using `if (!job.hasInputFile())` and requested to be reminded about it in a later PR.
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📚 Learning: 2026-01-21T14:59:52.530Z
Learnt from: kaligrafy
Repo: chairemobilite/transition PR: 1592
File: packages/transition-frontend/src/components/map/CircleSpinnerExtension.tsx:37-44
Timestamp: 2026-01-21T14:59:52.530Z
Learning: In TSX files under packages/transition-frontend/src that currently rely on deck.gl's this.context.animationProps.time for animation timing, prefer window.performance.now() (or performance.now()) to measure elapsed time. Replace time-based calculations tied to deck.gl with direct high-resolution timestamps to improve accuracy and decouple from the rendering library. Ensure animations use delta = now - lastTime, and update lastTime accordingly for smooth, frame-rate-independent timing.
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx (2)
171-179:⚠️ Potential issue | 🟡 MinorUse dots after the namespace in the tooltip key.
Line 178 currently looks up
transitPath:sort:TravelTime, so the new locale entry will not resolve and the title falls back to the raw key.🔧 Proposed fix
- title={props.t('transit:transitPath:sort:TravelTime')} + title={props.t('transit:transitPath.sort.TravelTime')}#!/bin/bash set -euo pipefail # Expect the locale files to contain the dot-form nested key, # and the component to still contain the colon-form lookup. rg -n "transitPath.sort.TravelTime|transit:transitPath:sort:TravelTime|transit:transitPath.sort.TravelTime" \ locales/en/transit.json \ locales/fr/transit.json \ packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx # No output here means the shared i18next config is not overriding the default separators. rg -n "keySeparator|nsSeparator" \ packages/chaire-lib-frontend/src/config/i18n.config.ts \ packages/chaire-lib-backend/src/config/i18next.ts || true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx` around lines 171 - 179, The tooltip i18n key uses colon separators for the whole path so the lookup fails; update the title prop on the Button in RoutingResultComponent (the JSX Button instance rendering the sort control) to use the namespace colon only and dots for the nested key (e.g., change the string passed to props.t from the colon-delimited path to "transit:transitPath.sort.TravelTime") so it matches the locale JSON nesting and resolves correctly.
94-114:⚠️ Potential issue | 🟠 MajorClamp
displayIndexbefore using it.If a new result has fewer alternatives, Line 103 can become
undefined, and Line 112 then callsshowCurrentAlternativewith an invalid index. This needs a synchronous clamp; a later reset effect still renders the bad index once.💡 Proposed fix
const result = props.result; const error = result.getError(); const alternativesCount = result.getAlternativesCount(); + const safeDisplayIndex = + alternativesCount === 0 ? 0 : Math.min(displayIndex, alternativesCount - 1); // Compute sorted indices when sorting is active const sortedIndices = React.useMemo( () => (sortedByDuration ? buildSortedIndices(result) : null), [result, sortedByDuration] ); // Map display position to the actual alternative index - const alternativeIndex = sortedIndices ? sortedIndices[displayIndex] : displayIndex; + const alternativeIndex = sortedIndices?.[safeDisplayIndex] ?? safeDisplayIndex; + + useEffect(() => { + if (displayIndex !== safeDisplayIndex) { + setDisplayIndex(safeDisplayIndex); + } + }, [displayIndex, safeDisplayIndex]); // Use effect to show the current alternative and fit bounds on initial render // Must be placed before any early returns to ensure hooks are called in the same order useEffect(() => { - if (error) { + if (error || alternativesCount === 0) { return; // Skip showing alternative if there's an error } const shouldFitBounds = isInitialRenderRef.current; showCurrentAlternative(result, alternativeIndex, shouldFitBounds); isInitialRenderRef.current = false; - }, [result, alternativeIndex, error]); + }, [result, alternativeIndex, alternativesCount, error]);- {displayIndex + 1}/{alternativesCount} + {safeDisplayIndex + 1}/{alternativesCount}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx` around lines 94 - 114, The effect can call showCurrentAlternative with an out-of-range index when the new result has fewer alternatives; before computing alternativeIndex, synchronously clamp displayIndex against the current alternativesCount (use alternativesCount from result.getAlternativesCount()) to produce a clampedDisplayIndex in-range (handle zero alternatives by skipping showCurrentAlternative), then use sortedIndices ? sortedIndices[clampedDisplayIndex] : clampedDisplayIndex when deriving alternativeIndex and pass that to showCurrentAlternative; update the useEffect dependency to use the clampedDisplayIndex/alternativeIndex so the hook always uses a valid index.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx`:
- Around line 171-179: The tooltip i18n key uses colon separators for the whole
path so the lookup fails; update the title prop on the Button in
RoutingResultComponent (the JSX Button instance rendering the sort control) to
use the namespace colon only and dots for the nested key (e.g., change the
string passed to props.t from the colon-delimited path to
"transit:transitPath.sort.TravelTime") so it matches the locale JSON nesting and
resolves correctly.
- Around line 94-114: The effect can call showCurrentAlternative with an
out-of-range index when the new result has fewer alternatives; before computing
alternativeIndex, synchronously clamp displayIndex against the current
alternativesCount (use alternativesCount from result.getAlternativesCount()) to
produce a clampedDisplayIndex in-range (handle zero alternatives by skipping
showCurrentAlternative), then use sortedIndices ?
sortedIndices[clampedDisplayIndex] : clampedDisplayIndex when deriving
alternativeIndex and pass that to showCurrentAlternative; update the useEffect
dependency to use the clampedDisplayIndex/alternativeIndex so the hook always
uses a valid index.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d0e50c88-49b6-486b-acf1-4025df8aa154
📒 Files selected for processing (3)
locales/en/transit.jsonlocales/fr/transit.jsonpackages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.json
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
Translation files in
locales/directory should organize strings by namespace (auth.json, transit.json, main.json, etc.) with support for both English (en/) and French (fr/) languages
Files:
locales/fr/transit.jsonlocales/en/transit.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
**/*.{ts,tsx}: Use UUIDs as primary keys for most entities in the database
Use the Status/Result pattern from chaire-lib-common for service functions and handlers
Indent using 4 spaces
Do not use trailing spaces, even in comments or documentation; replace lines with only spaces with blank lines
Use parentheses for arrow function parameters
Document non-trivial functions and attributes using JSDoc or inline comments when short
Use Geographic data in GeoJSON format with coordinates as [longitude, latitude]
Use i18next and thet()function for all user-facing strings to support internationalization
Files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
packages/transition-frontend/src/components/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
React components should use Redux for state management, i18next for translations, and SCSS for styling
Files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2026-02-03T20:40:36.982Z
Learning: Applies to packages/transition-frontend/src/components/routers/*.tsx : Application routing should be defined in `TransitionRouter.tsx` using React Router with feature-based route organization
📚 Learning: 2026-02-03T20:40:36.982Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2026-02-03T20:40:36.982Z
Learning: Applies to **/*.json : Translation files in `locales/` directory should organize strings by namespace (auth.json, transit.json, main.json, etc.) with support for both English (en/) and French (fr/) languages
Applied to files:
locales/fr/transit.jsonlocales/en/transit.json
📚 Learning: 2026-02-03T20:40:36.982Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2026-02-03T20:40:36.982Z
Learning: Applies to packages/transition-frontend/src/components/routers/*.tsx : Application routing should be defined in `TransitionRouter.tsx` using React Router with feature-based route organization
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📚 Learning: 2026-02-03T20:40:36.982Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2026-02-03T20:40:36.982Z
Learning: Applies to packages/transition-frontend/src/components/**/*.tsx : React components should use Redux for state management, i18next for translations, and SCSS for styling
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📚 Learning: 2026-02-03T20:40:36.982Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2026-02-03T20:40:36.982Z
Learning: Applies to **/*.{ts,tsx} : Use i18next and the `t()` function for all user-facing strings to support internationalization
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📚 Learning: 2025-09-25T16:09:00.577Z
Learnt from: greenscientist
Repo: chairemobilite/transition PR: 1494
File: packages/transition-backend/src/services/transitRouting/TrAccessibilityMapBatch.ts:42-45
Timestamp: 2025-09-25T16:09:00.577Z
Learning: In packages/transition-backend/src/services/transitRouting/TrAccessibilityMapBatch.ts, greenscientist deferred adding input file validation using `if (!job.hasInputFile())` and requested to be reminded about it in a later PR.
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📚 Learning: 2026-02-03T20:40:36.982Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2026-02-03T20:40:36.982Z
Learning: Applies to **/*.{ts,tsx} : Use the Status/Result pattern from chaire-lib-common for service functions and handlers
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📚 Learning: 2025-10-20T18:31:07.612Z
Learnt from: greenscientist
Repo: chairemobilite/transition PR: 1516
File: packages/chaire-lib-backend/src/utils/trRouting/TrRoutingServiceBackend.ts:29-41
Timestamp: 2025-10-20T18:31:07.612Z
Learning: In TrRoutingServiceBackend (packages/chaire-lib-backend/src/utils/trRouting/TrRoutingServiceBackend.ts), the fetch-retry package was not working as expected, so custom retry logic was implemented instead.
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📚 Learning: 2026-01-21T14:59:52.530Z
Learnt from: kaligrafy
Repo: chairemobilite/transition PR: 1592
File: packages/transition-frontend/src/components/map/CircleSpinnerExtension.tsx:37-44
Timestamp: 2026-01-21T14:59:52.530Z
Learning: In TSX files under packages/transition-frontend/src that currently rely on deck.gl's this.context.animationProps.time for animation timing, prefer window.performance.now() (or performance.now()) to measure elapsed time. Replace time-based calculations tied to deck.gl with direct high-resolution timestamps to improve accuracy and decouple from the rendering library. Ensure animations use delta = now - lastTime, and update lastTime accordingly for smooth, frame-rate-independent timing.
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
tahini
left a comment
tahini
left a comment
There was a problem hiding this comment.
Juste mettre à jour l'utilisation du hook vs hoc pour les traductions du composants.
| } | ||
|
|
||
| export interface TransitRoutingResultsProps { | ||
| export interface TransitRoutingResultsProps extends WithTranslation { |
There was a problem hiding this comment.
La façon de faire actuelle de react est de plutôt utiliser les hooks que les hocs, surtout si le composant est un fucntion component, comme c'est le cas. Par exemple, ici, au lieu d'extend WithTranslation, il faut plutôt mettre dans le composant const { t } = useTranslation('transit'). Notre vieux code utilise encore souvent le WithTranslation, mais le nouveau doit se conformer aux nouvelles façons de faire.
Closes chairemobilite#1511 Add a toggle button to the routing results navigation that reorders alternative paths by ascending travel time. When active (green), the shortest trip appears first. Toggling off reverts to the original order. Co-authored-by: Rémi Labelle <remi-2.labelle@polymtl.ca>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx (1)
87-104:⚠️ Potential issue | 🟠 Major
displayIndexshould be reset whenresultchanges.If a new routing result has fewer alternatives,
displayIndexmay exceed the valid range, causingsortedIndices[displayIndex]to beundefined. This was flagged in a previous review but doesn't appear to be addressed yet.Proposed fix
const [displayIndex, setDisplayIndex] = useState(0); // FIXME: Store in user preferences const [sortedByDuration, setSortedByDuration] = useState(false); // Track if this is the first render to fit bounds only on initial display const isInitialRenderRef = useRef(true); const result = props.result; const error = result.getError(); const alternativesCount = result.getAlternativesCount(); + // Reset display index when the routing result changes + useEffect(() => { + setDisplayIndex(0); + isInitialRenderRef.current = true; + }, [result]); + // Compute sorted indices when sorting is active🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx` around lines 87 - 104, When the incoming routing result changes, reset or clamp the displayIndex so it cannot point past the available alternatives; add a React useEffect that watches result (or alternativesCount/result.getAlternativesCount()) and calls setDisplayIndex(0) or setDisplayIndex(prev => Math.min(prev, Math.max(0, result.getAlternativesCount() - 1))) to ensure alternativeIndex (computed from sortedIndices and displayIndex) is never undefined; place the effect near the existing state hooks referencing displayIndex/sortedIndices to keep behavior consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx`:
- Around line 87-104: When the incoming routing result changes, reset or clamp
the displayIndex so it cannot point past the available alternatives; add a React
useEffect that watches result (or
alternativesCount/result.getAlternativesCount()) and calls setDisplayIndex(0) or
setDisplayIndex(prev => Math.min(prev, Math.max(0, result.getAlternativesCount()
- 1))) to ensure alternativeIndex (computed from sortedIndices and displayIndex)
is never undefined; place the effect near the existing state hooks referencing
displayIndex/sortedIndices to keep behavior consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d75caab3-06f9-411d-ad65-3b492a96b650
📒 Files selected for processing (3)
locales/en/transit.jsonlocales/fr/transit.jsonpackages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.json
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
Translation files in
locales/directory should organize strings by namespace (auth.json, transit.json, main.json, etc.) with support for both English (en/) and French (fr/) languages
Files:
locales/en/transit.jsonlocales/fr/transit.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
**/*.{ts,tsx}: Use UUIDs as primary keys for most entities in the database
Use the Status/Result pattern from chaire-lib-common for service functions and handlers
Indent using 4 spaces
Do not use trailing spaces, even in comments or documentation; replace lines with only spaces with blank lines
Use parentheses for arrow function parameters
Document non-trivial functions and attributes using JSDoc or inline comments when short
Use Geographic data in GeoJSON format with coordinates as [longitude, latitude]
Use i18next and thet()function for all user-facing strings to support internationalization
Files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
packages/transition-frontend/src/components/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
React components should use Redux for state management, i18next for translations, and SCSS for styling
Files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
🧠 Learnings (10)
📚 Learning: 2026-02-03T20:40:36.982Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2026-02-03T20:40:36.982Z
Learning: Applies to **/*.json : Translation files in `locales/` directory should organize strings by namespace (auth.json, transit.json, main.json, etc.) with support for both English (en/) and French (fr/) languages
Applied to files:
locales/en/transit.jsonlocales/fr/transit.json
📚 Learning: 2026-02-03T20:40:36.982Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2026-02-03T20:40:36.982Z
Learning: Applies to packages/transition-frontend/src/components/routers/*.tsx : Application routing should be defined in `TransitionRouter.tsx` using React Router with feature-based route organization
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📚 Learning: 2026-02-03T20:40:36.982Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2026-02-03T20:40:36.982Z
Learning: Applies to packages/transition-frontend/src/components/**/*.tsx : React components should use Redux for state management, i18next for translations, and SCSS for styling
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📚 Learning: 2026-02-03T20:40:36.982Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2026-02-03T20:40:36.982Z
Learning: Applies to **/*.{ts,tsx} : Use i18next and the `t()` function for all user-facing strings to support internationalization
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📚 Learning: 2025-09-25T16:09:00.577Z
Learnt from: greenscientist
Repo: chairemobilite/transition PR: 1494
File: packages/transition-backend/src/services/transitRouting/TrAccessibilityMapBatch.ts:42-45
Timestamp: 2025-09-25T16:09:00.577Z
Learning: In packages/transition-backend/src/services/transitRouting/TrAccessibilityMapBatch.ts, greenscientist deferred adding input file validation using `if (!job.hasInputFile())` and requested to be reminded about it in a later PR.
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📚 Learning: 2026-02-03T20:40:36.982Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2026-02-03T20:40:36.982Z
Learning: Applies to **/*.{ts,tsx} : Use the Status/Result pattern from chaire-lib-common for service functions and handlers
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📚 Learning: 2025-10-20T18:31:07.612Z
Learnt from: greenscientist
Repo: chairemobilite/transition PR: 1516
File: packages/chaire-lib-backend/src/utils/trRouting/TrRoutingServiceBackend.ts:29-41
Timestamp: 2025-10-20T18:31:07.612Z
Learning: In TrRoutingServiceBackend (packages/chaire-lib-backend/src/utils/trRouting/TrRoutingServiceBackend.ts), the fetch-retry package was not working as expected, so custom retry logic was implemented instead.
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📚 Learning: 2026-01-28T18:01:07.845Z
Learnt from: kaligrafy
Repo: chairemobilite/transition PR: 1736
File: packages/transition-backend/src/services/generalizedCostFunction/GeneralizedCostFunction.ts:54-63
Timestamp: 2026-01-28T18:01:07.845Z
Learning: In packages/transition-backend/src/services/generalizedCostFunction/GeneralizedCostFunction.ts, the headway thresholds in getHeadwayThresholdSeconds() are intentionally hard-coded constants rather than configurable parameters. These values should be based on international consensus to allow consistent comparison of transit service quality across different cities. The TODO comments indicate that these values need more research and community discussion before being finalized.
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📚 Learning: 2025-11-04T19:35:10.045Z
Learnt from: tahini
Repo: chairemobilite/transition PR: 1554
File: packages/transition-common/src/services/simulation/simulationMethod/OdTripSimulationMethod.ts:23-36
Timestamp: 2025-11-04T19:35:10.045Z
Learning: In the OdTripSimulationMethod (packages/transition-common/src/services/simulation/simulationMethod/OdTripSimulationMethod.ts), the team plans to move away from the datasource path and use batch calculations instead. The current non-functional dataSourceId selector will be reworked as part of this architectural change.
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
📚 Learning: 2026-01-21T14:59:52.530Z
Learnt from: kaligrafy
Repo: chairemobilite/transition PR: 1592
File: packages/transition-frontend/src/components/map/CircleSpinnerExtension.tsx:37-44
Timestamp: 2026-01-21T14:59:52.530Z
Learning: In TSX files under packages/transition-frontend/src that currently rely on deck.gl's this.context.animationProps.time for animation timing, prefer window.performance.now() (or performance.now()) to measure elapsed time. Replace time-based calculations tied to deck.gl with direct high-resolution timestamps to improve accuracy and decouple from the rendering library. Ensure animations use delta = now - lastTime, and update lastTime accordingly for smooth, frame-rate-independent timing.
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx
🔇 Additional comments (8)
locales/fr/transit.json (1)
440-442: LGTM!Translation key is properly nested under
transitPath.sortand the French text is appropriate.locales/en/transit.json (1)
439-441: LGTM!Translation key is properly structured and the English text is clear.
packages/transition-frontend/src/components/forms/transitRouting/RoutingResultComponent.tsx (6)
10-10: LGTM!Good choice of icon for the sort button.
22-23: LGTM!Using
useTranslationhook instead of the HOC pattern follows the modern React approach.
25-43: LGTM!Helper functions are well-structured. The FIXME comments clearly indicate future refactoring plans.
123-134: LGTM!Navigation handlers correctly manage wraparound and the sort toggle properly resets the display index.
172-181: LGTM!Sort button has proper i18n title for accessibility and the color toggle provides clear visual feedback.
160-160: LGTM!Good choice showing
displayIndex + 1for the user-facing counter - it reflects the visual position regardless of sorting.
tahini
left a comment
tahini
left a comment
There was a problem hiding this comment.
C,est bon, le changement est scopé dans un seul fichier, donc facile à identifier et éventuellement étendre.
4 participants
Closes #1511
Add a toggle button to the routing results navigation that reorders alternative paths by ascending travel time. When active (green), the shortest trip appears first. Toggling off reverts to the original order.
Summary by CodeRabbit
New Features
Documentation