objects: add a history tracker and have forms use the useHistoryTracker hook#1565
objects: add a history tracker and have forms use the useHistoryTracker hook#1565tahini wants to merge 3 commits intochairemobilite:mainfrom
useHistoryTracker hook#1565Conversation
WalkthroughAdds a generic immutable HistoryTracker class that records bounded deep-cloned object snapshots and exposes canUndo/canRedo/undo/redo/current/reset/record/hasChanged. Introduces a React hook useHistoryTracker and exported type WithHistoryTracker to track an object's attributes, formValues, and invalid-field state, backed by HistoryTracker, and exposes onValueChange, undo/redo, canUndo/canRedo, hasInvalidFields, updateHistory, and formValues. Adds an UndoRedoButtons UI component. Refactors TransitRoutingForm to use useHistoryTracker and HistoryTracker (replacing prior local/history logic), integrates undo/redo into the form flow, adjusts lifecycle/map-layer update handling, and adds unit tests for HistoryTracker and the hook. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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/TransitRoutingForm.tsx (1)
124-308: Validate against the updated TransitRouting instance
isValid()closes over the oldtransitRoutingobject. WhenupdateCurrentObjectassignstransitRoutingRef.current = newObject;and then callsisValid(), the check still interrogates the pre-undo/redo instance. That means we can skipupdateRoutingPrefs()when undoing back to a valid state (old object was invalid) or run it even though the new object is invalid (old one was valid).Please have the validity check use the new object (and avoid the stale closure) before updating preferences. While touching that block, switching
setChangeCountto a functional update avoids the rare lost increment if this helper is ever called twice before the next render.- const isValid = (): boolean => { - // Are all form fields valid and the routing object too - return !hasInvalidFields() && transitRouting.isValid(); - }; + const isValid = (): boolean => { + return !hasInvalidFields() && transitRoutingRef.current.isValid(); + }; ... - setChangeCount(changeCount + 1); - // Update routing preferences if the object is valid. - // FIXME Should we calculate too? - if (isValid()) { + setChangeCount((count) => count + 1); + if (!hasInvalidFields() && newObject.isValid()) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
packages/chaire-lib-common/src/utils/objects/HistoryTracker.ts(1 hunks)packages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts(1 hunks)packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsx(1 hunks)packages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsx(1 hunks)packages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsx(1 hunks)packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx(12 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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/TransitRoutingForm.tsx
🧬 Code graph analysis (5)
packages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsx (1)
packages/chaire-lib-frontend/src/components/input/Button.tsx (1)
Button(38-138)
packages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts (1)
packages/chaire-lib-common/src/utils/objects/HistoryTracker.ts (1)
HistoryTracker(13-111)
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx (6)
packages/transition-common/src/services/transitRouting/TransitRouting.ts (2)
TransitRouting(44-243)TransitRoutingAttributes(40-40)packages/chaire-lib-common/src/services/routing/types.ts (1)
RoutingResultsByMode(75-79)packages/chaire-lib-common/src/utils/TrError.ts (1)
ErrorMessage(14-14)packages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsx (1)
useHistoryTracker(92-239)packages/chaire-lib-common/src/services/events/EventManager.ts (1)
EventManager(21-58)packages/chaire-lib-frontend/src/services/map/events/MapEventsCallbacks.ts (1)
MapUpdateLayerEventType(8-14)
packages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsx (2)
packages/chaire-lib-common/src/utils/objects/HistoryTracker.ts (1)
HistoryTracker(13-111)packages/chaire-lib-common/src/utils/objects/ObjectWithHistory.ts (4)
undo(57-61)canUndo(49-51)redo(63-67)canRedo(53-55)
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsx (1)
packages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsx (1)
useHistoryTracker(92-239)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-and-run-UI-tests (24.x)
- GitHub Check: code-lint
- GitHub Check: build-and-test (24.x)
- GitHub Check: pr-build-check
| useEffect( | ||
| () => { | ||
| const trackedAttributes = { | ||
| attributes: object.attributes, | ||
| formValues: _cloneDeep(object.attributes), | ||
| invalidFields: {} | ||
| }; | ||
| // Create a new history tracker and reset fields when the object id changes | ||
| const newHistoryTracker = new HistoryTracker(trackedAttributes); | ||
| setHistoryTracker(newHistoryTracker); | ||
|
|
||
| // Reset the invalidFields ref when object ID changes | ||
| invalidFieldsRef.current = {}; | ||
| setInvalidFieldsCount(0); | ||
|
|
||
| // Also reset the ref when object ID changes | ||
| formValuesRef.current = _cloneDeep(object.attributes); | ||
| setFormValues(_cloneDeep(object.attributes)); | ||
| }, | ||
| (object as any).id !== undefined ? [(object as any).id] : [] | ||
| ); |
There was a problem hiding this comment.
Fix history reset for unsaved objects
If object.id is undefined, the dependency array ends up empty, so swapping in a brand-new unsaved object never re-runs the effect. The hook then keeps the previous form values and history for the wrong object. Please track a stable fallback (e.g. the object reference) so the effect resets whenever a new unsaved object comes in.
- useEffect(
- () => {
+ const historyTrackerKey = (object as any).id ?? object;
+
+ useEffect(
+ () => {
const trackedAttributes = {
attributes: object.attributes,
formValues: _cloneDeep(object.attributes),
invalidFields: {}
};
// Create a new history tracker and reset fields when the object id changes
const newHistoryTracker = new HistoryTracker(trackedAttributes);
setHistoryTracker(newHistoryTracker);
@@
formValuesRef.current = _cloneDeep(object.attributes);
setFormValues(_cloneDeep(object.attributes));
},
- (object as any).id !== undefined ? [(object as any).id] : []
+ [historyTrackerKey]
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect( | |
| () => { | |
| const trackedAttributes = { | |
| attributes: object.attributes, | |
| formValues: _cloneDeep(object.attributes), | |
| invalidFields: {} | |
| }; | |
| // Create a new history tracker and reset fields when the object id changes | |
| const newHistoryTracker = new HistoryTracker(trackedAttributes); | |
| setHistoryTracker(newHistoryTracker); | |
| // Reset the invalidFields ref when object ID changes | |
| invalidFieldsRef.current = {}; | |
| setInvalidFieldsCount(0); | |
| // Also reset the ref when object ID changes | |
| formValuesRef.current = _cloneDeep(object.attributes); | |
| setFormValues(_cloneDeep(object.attributes)); | |
| }, | |
| (object as any).id !== undefined ? [(object as any).id] : [] | |
| ); | |
| const historyTrackerKey = (object as any).id ?? object; | |
| useEffect( | |
| () => { | |
| const trackedAttributes = { | |
| attributes: object.attributes, | |
| formValues: _cloneDeep(object.attributes), | |
| invalidFields: {} | |
| }; | |
| // Create a new history tracker and reset fields when the object id changes | |
| const newHistoryTracker = new HistoryTracker(trackedAttributes); | |
| setHistoryTracker(newHistoryTracker); | |
| // Reset the invalidFields ref when object ID changes | |
| invalidFieldsRef.current = {}; | |
| setInvalidFieldsCount(0); | |
| // Also reset the ref when object ID changes | |
| formValuesRef.current = _cloneDeep(object.attributes); | |
| setFormValues(_cloneDeep(object.attributes)); | |
| }, | |
| [historyTrackerKey] | |
| ); |
🤖 Prompt for AI Agents
In packages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsx
around lines 112 to 132, the useEffect dependency array becomes empty when
object.id is undefined so the effect doesn't re-run for new unsaved objects;
change the dependency to include a stable fallback such as the object reference
(e.g. use (object as any).id ?? object) so the effect resets the HistoryTracker,
invalidFieldsRef, formValuesRef and related state whenever either the persisted
id changes or a new unsaved object instance is passed in.
There was a problem hiding this comment.
@kaligrafy Thanks for confirming! The suggested change should help ensure the history tracker properly resets when switching between unsaved objects.
There was a problem hiding this comment.
No this is fine actually. Since undo/redo creates a new object, the suggestion will cause this effect to re-run whenever those functions are executed, effectively allowing only one undo/redo per object without ID. In the code base, object classes that have an ID field will always have this field defined. But in case some object has an actually undefined id that can be defined later, it's safe to use [(object as any).id] for all cases: object without ids will never retrigger the effect and if the ID is eventually set, it will retrigger.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| useEffect( | ||
| () => { | ||
| const trackedAttributes = { | ||
| attributes: object.attributes, | ||
| formValues: _cloneDeep(object.attributes), | ||
| invalidFields: {} | ||
| }; | ||
| // Create a new history tracker and reset fields when the object id changes | ||
| const newHistoryTracker = new HistoryTracker(trackedAttributes); | ||
| setHistoryTracker(newHistoryTracker); | ||
|
|
||
| // Reset the invalidFields ref when object ID changes | ||
| invalidFieldsRef.current = {}; | ||
| setInvalidFieldsCount(0); | ||
|
|
||
| // Also reset the ref when object ID changes | ||
| formValuesRef.current = _cloneDeep(object.attributes); | ||
| setFormValues(_cloneDeep(object.attributes)); | ||
| }, | ||
| (object as any).id !== undefined ? [(object as any).id] : [] | ||
| ); |
There was a problem hiding this comment.
Actionable comments posted: 5
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/TransitRoutingForm.tsx (1)
109-126: Double mutation of object attribute.
onFieldValueChange(fromuseHistoryTracker) already callsobject.set(path, newValue.value)for valid values (see lines 158-159 in useHistoryTracker.tsx). The additionalupdatedObject.set(path, newValue.value)on line 118 causes a redundant mutation.🔧 Suggested fix
const onValueChange = useCallback(( path: keyof TransitRoutingAttributes, newValue: { value: any; valid?: boolean } = { value: null, valid: true }, resetResults = true ) => { setRoutingErrors([]); //When a value is changed, remove the current routingErrors to stop displaying them. onFieldValueChange(path, newValue); - if (newValue.valid || newValue.valid === undefined) { - const updatedObject = transitRouting; - updatedObject.set(path, newValue.value); - setRoutingAttributes({ ...updatedObject.attributes }); - } + // Trigger re-render after the change + setRoutingAttributes({ ...transitRouting.attributes }); if (resetResults) { resetResultsData(); } updateHistory(); }, [onFieldValueChange, resetResultsData, transitRouting, updateHistory]);
🤖 Fix all issues with AI agents
In @packages/chaire-lib-common/src/utils/objects/HistoryTracker.ts:
- Around line 56-60: current() on HistoryTracker returns the direct object from
history[this.historyIndex], exposing internal state to external mutation; change
current() to return a cloned copy (the same way undo() and redo() do) so callers
receive an immutable snapshot. Locate the HistoryTracker class and the current
method, and replace the direct return of history[this.historyIndex] with a
deep-clone of that entry (using the project's established clone utility or the
same cloning approach used by undo()/redo()) so the internal history array
cannot be mutated by consumers.
In @packages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsx:
- Around line 34-59: The JSX wraps each single <Button> in unnecessary
curly-brace expression fragments; remove the surrounding { ... } so the elements
are rendered directly. Locate the two Button instances (title={t('main:Undo')}
name="undo" key="undo" ... onClick={props.onUndo} and title={t('main:Redo')}
name="redo" key="redo" ... onClick={props.onRedo}) and replace the wrapped
expressions with the raw <Button ... /> elements, keeping all props and handlers
(props.canUndo, props.canRedo, props.onUndo, props.onRedo) unchanged.
- Around line 29-64: The component function is named SelectedObjectButtons while
the file is UndoRedoButtons.tsx; rename the function and its default export from
SelectedObjectButtons to UndoRedoButtons (update the declaration const
SelectedObjectButtons: React.FunctionComponent<UndoRedoButtonsProps> = ... to
const UndoRedoButtons: React.FunctionComponent<UndoRedoButtonsProps> = ... and
change export default SelectedObjectButtons to export default UndoRedoButtons)
and search/update any imports or references to the old name to match the new
UndoRedoButtons identifier; optionally set component.displayName =
'UndoRedoButtons' for clearer debugging.
In
@packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx:
- Around line 316-321: The call to isValid() can capture a stale transitRouting
via the closure; instead use the new object directly: replace the isValid()
check inside updateCurrentObject with newObject.isValid() (or call
newObject.hasInvalidFields()/transitRouting.isValid() on the newObject if
needed) so the validation uses the freshly-updated transitRoutingRef.current
before invoking newObject.updateRoutingPrefs(); ensure you still increment
setChangeCount(changeCount + 1) and only call newObject.updateRoutingPrefs()
when newObject.isValid().
- Around line 313-336: The handlers updateCurrentObject, onUndo, and onRedo are
recreated each render and should be memoized with useCallback to avoid needless
re-renders; wrap updateCurrentObject in useCallback and include its dependencies
(resetResultsData, setChangeCount, changeCount, isValid, and transitRoutingRef
as needed), then memoize onUndo and onRedo with useCallback referencing
undo/redo and updateCurrentObject; ensure the dependency arrays include the
exact functions/values used (e.g., undo, redo, updateCurrentObject) so they
update when those change.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
packages/chaire-lib-common/src/utils/objects/HistoryTracker.tspackages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.tspackages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsxpackages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsxpackages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
**/*.{ts,tsx}: Use TypeScript strictly - avoidanytypes in all files
All geographic data must use GeoJSON format with coordinates as [longitude, latitude]
Document non-trivial functions and attributes using JSDoc or inline comments when short
Use parentheses for all arrow function parameters, even single parameters
Use @turf/turf and geojson libraries for geospatial calculations and GeoJSON type definitions
Frontend build process requires building frontend withyarn build:devoryarn build:prodbefore starting backend server
Files:
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsxpackages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsxpackages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.tspackages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsxpackages/chaire-lib-common/src/utils/objects/HistoryTracker.ts
**/*.{ts,tsx,js,jsx,json,scss,sass}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
Indent using 4 spaces throughout all code files
Files:
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsxpackages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsxpackages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.tspackages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsxpackages/chaire-lib-common/src/utils/objects/HistoryTracker.ts
**/*.{ts,tsx,js,jsx,json,scss,sass,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
No trailing spaces in any lines, including comments and documentation; replace lines with spaces only with blank lines
Files:
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsxpackages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsxpackages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.tspackages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsxpackages/chaire-lib-common/src/utils/objects/HistoryTracker.ts
**/__tests__/**/*.{test,spec}.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
**/__tests__/**/*.{test,spec}.{ts,tsx,js}: Use parametric tests in Jest when more than one expect statement is used to simplify debugging of failed tests
Testing framework must use Jest for unit tests and Playwright for UI tests; test files located in tests/ directories
Files:
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts
**/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
All user-facing strings must use i18next internationalization with the
t()function
Files:
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsxpackages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsxpackages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsx
**/*.{ts,tsx,js,jsx,json,scss,sass,md}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
Code formatting must follow Prettier configuration from configs/ directory; run
yarn formatbefore committing
Files:
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsxpackages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsxpackages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.tspackages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsxpackages/chaire-lib-common/src/utils/objects/HistoryTracker.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
All code must be linted with
yarn lintand formatted withyarn formatbefore committing
Files:
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsxpackages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsxpackages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.tspackages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsxpackages/chaire-lib-common/src/utils/objects/HistoryTracker.ts
**/transition-frontend/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
Frontend React components should be organized by feature in packages/transition-frontend/src/components/ with dedicated subdirectories for forms, dashboard, map components, etc.
Files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
**/transition-frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
Use Redux with Redux Thunk for state management in React components
Files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
**/__tests__/**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
Sequential/integration tests require separate test database setup with
yarn setup-test && yarn migrate-testbefore running
Files:
packages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts
🧠 Learnings (17)
📓 Common learnings
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/transition-frontend/src/**/*.{ts,tsx} : Use Redux with Redux Thunk for state management in React components
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/__tests__/**/*.{test,spec}.{ts,tsx,js} : Use parametric tests in Jest when more than one expect statement is used to simplify debugging of failed tests
Applied to files:
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts
📚 Learning: 2025-12-17T22:46:52.336Z
Learnt from: tahini
Repo: chairemobilite/transition PR: 1666
File: packages/chaire-lib-frontend/package.json:60-60
Timestamp: 2025-12-17T22:46:52.336Z
Learning: In packages/chaire-lib-frontend/package.json, the `testing-library/dom` devDependency is required for tests to pass even though it's not explicitly imported. It was previously provided as a transitive dependency through `react-select-event`, but after removing `react-select-event`, it must be explicitly declared.
Applied to files:
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/__tests__/**/*.{test,spec}.{ts,tsx,js} : Testing framework must use Jest for unit tests and Playwright for UI tests; test files located in __tests__/ directories
Applied to files:
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/transition-frontend/src/components/routers/TransitionRouter.tsx : Frontend routing must be defined in packages/transition-frontend/src/components/routers/TransitionRouter.tsx
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/transition-backend/src/services/transitRouting/**/*.ts : Transit routing calculations must be implemented using the service in packages/transition-backend/src/services/transitRouting/
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Transition is a full-stack TypeScript/React application for transit planning and simulation; always consider the complete architecture of modeling, simulating, and planning features
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/transition-frontend/src/components/**/*.{ts,tsx} : Frontend React components should be organized by feature in packages/transition-frontend/src/components/ with dedicated subdirectories for forms, dashboard, map components, etc.
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/transition-frontend/src/app-transition.tsx : Frontend application entry point is packages/transition-frontend/src/app-transition.tsx
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/transition-frontend/src/**/*.{ts,tsx} : Use Redux with Redux Thunk for state management in React components
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsxpackages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.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/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/src/utils/processManagers/**/*.ts : Services using external routing engines must configure OSRM for road-based modes (walking, cycling, driving, bus) and trRouting for transit routing
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/transition-backend/src/services/gtfsImport/**/*.ts : GTFS import functionality must be implemented following the workflow in packages/transition-backend/src/services/gtfsImport/
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/src/models/db/Transit*.ts : Transit objects follow a hierarchy: Agency > Lines > Services > Paths > Nodes; maintain this structure when designing data models
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Ensure trRouting service is running and scenario is loaded before attempting transit routing calculations
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Scenarios represent complete transit network configurations; always ensure scenario consistency when modifying transit objects
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/__tests__/**/*.test.ts : Sequential/integration tests require separate test database setup with `yarn setup-test && yarn migrate-test` before running
Applied to files:
packages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts
🧬 Code graph analysis (3)
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsx (1)
packages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsx (1)
useHistoryTracker(92-237)
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx (9)
packages/transition-common/src/services/transitRouting/TransitRouting.ts (2)
TransitRouting(44-243)TransitRoutingAttributes(40-40)packages/chaire-lib-common/src/services/routing/types.ts (1)
RoutingResultsByMode(75-79)packages/chaire-lib-common/src/config/routingModes.ts (1)
RoutingOrTransitMode(32-32)packages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsx (1)
useHistoryTracker(92-237)packages/chaire-lib-frontend/src/services/map/events/MapEventsCallbacks.ts (1)
MapUpdateLayerEventType(8-14)packages/chaire-lib-common/src/utils/objects/__tests__/GenericObjectCollection.test.ts (1)
newObject(16-19)packages/chaire-lib-common/src/utils/objects/GenericObject.ts (1)
isValid(63-65)packages/chaire-lib-common/src/utils/objects/ObjectWithHistory.ts (4)
undo(57-61)redo(63-67)canUndo(49-51)canRedo(53-55)packages/chaire-lib-frontend/src/components/input/InputWrapper.tsx (1)
InputWrapper(25-53)
packages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts (1)
packages/chaire-lib-common/src/utils/objects/HistoryTracker.ts (1)
HistoryTracker(13-111)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: code-lint
- GitHub Check: build-and-run-UI-tests (24.x)
- GitHub Check: pr-build-check
- GitHub Check: test-sequential (24.x)
- GitHub Check: build-and-test (24.x)
🔇 Additional comments (4)
packages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts (1)
1-162: Good test coverage for HistoryTracker.The test suite thoroughly covers initialization, undo/redo mechanics, optional fields, reset behavior, history limits, and change detection. Well organized!
Minor style note: a few closing braces are missing semicolons (lines 34, 122), but this is cosmetic.
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsx (1)
1-501: Excellent test coverage for the useHistoryTracker hook.Tests cover value changes (valid/invalid), undo/redo operations, atomic multi-field updates, invalid field tracking, and re-render behavior with various ID scenarios. Good work!
packages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsx (2)
181-200: Constructor assumption in undo/redo.The code assumes
object.constructoracceptsPartial<T['attributes']>as a single argument. This works for the current codebase but is worth documenting or validating if BaseObject subclasses have different constructor signatures.
92-236: Well-structured hook implementation.The hook cleanly separates concerns: form value tracking, invalid field management, and history operations. The use of refs for immediate access and state for re-renders is a good pattern.
The FIXME comments (lines 76-87, 93-94) acknowledge known limitations - good practice to track these.
packages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsx
Outdated
Show resolved
Hide resolved
| { | ||
| <Button | ||
| title={t('main:Undo')} | ||
| name="undo" | ||
| key="undo" | ||
| color="grey" | ||
| disabled={!props.canUndo()} | ||
| icon={faUndoAlt} | ||
| iconClass="_icon-alone" | ||
| label="" | ||
| onClick={props.onUndo} | ||
| /> | ||
| } | ||
| { | ||
| <Button | ||
| title={t('main:Redo')} | ||
| name="redo" | ||
| key="redo" | ||
| color="grey" | ||
| disabled={!props.canRedo()} | ||
| icon={faRedoAlt} | ||
| iconClass="_icon-alone" | ||
| label="" | ||
| onClick={props.onRedo} | ||
| /> | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unnecessary JSX fragments around buttons.
The curly braces around single <Button> elements are not needed.
♻️ Cleaner JSX
<div className="tr__form-buttons-container tr__form-selected-object-buttons-container">
- {
- <Button
- title={t('main:Undo')}
- ...
- />
- }
- {
- <Button
- title={t('main:Redo')}
- ...
- />
- }
+ <Button
+ title={t('main:Undo')}
+ name="undo"
+ key="undo"
+ color="grey"
+ disabled={!props.canUndo()}
+ icon={faUndoAlt}
+ iconClass="_icon-alone"
+ label=""
+ onClick={props.onUndo}
+ />
+ <Button
+ title={t('main:Redo')}
+ name="redo"
+ key="redo"
+ color="grey"
+ disabled={!props.canRedo()}
+ icon={faRedoAlt}
+ iconClass="_icon-alone"
+ label=""
+ onClick={props.onRedo}
+ />
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| <Button | |
| title={t('main:Undo')} | |
| name="undo" | |
| key="undo" | |
| color="grey" | |
| disabled={!props.canUndo()} | |
| icon={faUndoAlt} | |
| iconClass="_icon-alone" | |
| label="" | |
| onClick={props.onUndo} | |
| /> | |
| } | |
| { | |
| <Button | |
| title={t('main:Redo')} | |
| name="redo" | |
| key="redo" | |
| color="grey" | |
| disabled={!props.canRedo()} | |
| icon={faRedoAlt} | |
| iconClass="_icon-alone" | |
| label="" | |
| onClick={props.onRedo} | |
| /> | |
| } | |
| <Button | |
| title={t('main:Undo')} | |
| name="undo" | |
| key="undo" | |
| color="grey" | |
| disabled={!props.canUndo()} | |
| icon={faUndoAlt} | |
| iconClass="_icon-alone" | |
| label="" | |
| onClick={props.onUndo} | |
| /> | |
| <Button | |
| title={t('main:Redo')} | |
| name="redo" | |
| key="redo" | |
| color="grey" | |
| disabled={!props.canRedo()} | |
| icon={faRedoAlt} | |
| iconClass="_icon-alone" | |
| label="" | |
| onClick={props.onRedo} | |
| /> |
🤖 Prompt for AI Agents
In @packages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsx
around lines 34 - 59, The JSX wraps each single <Button> in unnecessary
curly-brace expression fragments; remove the surrounding { ... } so the elements
are rendered directly. Locate the two Button instances (title={t('main:Undo')}
name="undo" key="undo" ... onClick={props.onUndo} and title={t('main:Redo')}
name="redo" key="redo" ... onClick={props.onRedo}) and replace the wrapped
expressions with the raw <Button ... /> elements, keeping all props and handlers
(props.canUndo, props.canRedo, props.onUndo, props.onRedo) unchanged.
| const updateCurrentObject = (newObject: TransitRouting) => { | ||
| transitRoutingRef.current = newObject; | ||
| resetResultsData(); | ||
| setChangeCount(changeCount + 1); | ||
| // Update routing preferences if the object is valid. | ||
| // FIXME Should we calculate too? | ||
| if (isValid()) { | ||
| newObject.updateRoutingPrefs(); | ||
| } | ||
| }; | ||
|
|
||
| const onUndo = () => { | ||
| const newObject = undo(); | ||
| if (newObject) { | ||
| updateCurrentObject(newObject); | ||
| } | ||
| }; | ||
|
|
||
| const onRedo = () => { | ||
| const newObject = redo(); | ||
| if (newObject) { | ||
| updateCurrentObject(newObject); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider wrapping handlers in useCallback.
updateCurrentObject, onUndo, and onRedo are defined inside the component but not memoized. They reference state (changeCount) and other values, so they're recreated each render. This could cause unnecessary re-renders of child components receiving these as props.
♻️ Memoize handlers
- const updateCurrentObject = (newObject: TransitRouting) => {
+ const updateCurrentObject = useCallback((newObject: TransitRouting) => {
transitRoutingRef.current = newObject;
resetResultsData();
setChangeCount(changeCount + 1);
- // Update routing preferences if the object is valid.
- // FIXME Should we calculate too?
- if (isValid()) {
+ if (newObject.isValid()) {
newObject.updateRoutingPrefs();
}
- };
+ }, [changeCount, resetResultsData]);
- const onUndo = () => {
+ const onUndo = useCallback(() => {
const newObject = undo();
if (newObject) {
updateCurrentObject(newObject);
}
- };
+ }, [undo, updateCurrentObject]);
- const onRedo = () => {
+ const onRedo = useCallback(() => {
const newObject = redo();
if (newObject) {
updateCurrentObject(newObject);
}
- };
+ }, [redo, updateCurrentObject]);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
@packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
around lines 313 - 336, The handlers updateCurrentObject, onUndo, and onRedo are
recreated each render and should be memoized with useCallback to avoid needless
re-renders; wrap updateCurrentObject in useCallback and include its dependencies
(resetResultsData, setChangeCount, changeCount, isValid, and transitRoutingRef
as needed), then memoize onUndo and onRedo with useCallback referencing
undo/redo and updateCurrentObject; ensure the dependency arrays include the
exact functions/values used (e.g., undo, redo, updateCurrentObject) so they
update when those change.
| setChangeCount(changeCount + 1); | ||
| // Update routing preferences if the object is valid. | ||
| // FIXME Should we calculate too? | ||
| if (isValid()) { | ||
| newObject.updateRoutingPrefs(); | ||
| } |
There was a problem hiding this comment.
Potential stale closure with isValid().
isValid() references hasInvalidFields() and transitRouting.isValid(). When called inside updateCurrentObject, the transitRouting reference may be stale since transitRoutingRef.current was just updated but the closure still captures the old value. Consider using newObject.isValid() directly.
🤖 Prompt for AI Agents
In
@packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
around lines 316 - 321, The call to isValid() can capture a stale transitRouting
via the closure; instead use the new object directly: replace the isValid()
check inside updateCurrentObject with newObject.isValid() (or call
newObject.hasInvalidFields()/transitRouting.isValid() on the newObject if
needed) so the validation uses the freshly-updated transitRoutingRef.current
before invoking newObject.updateRoutingPrefs(); ensure you still increment
setChangeCount(changeCount + 1) and only call newObject.updateRoutingPrefs()
when newObject.isValid().
This allows to track the history of any object type, undo and redo changes. Nodejs has a few modules that provide similar functionality, but they are very little used and not typed see https://www.npmjs.com/search?q=keywords:undo-redo
… objects This is a React hook version of the current ChangeEventsForm. It tracks the valid and invalid changes to an object with numeric ID, storing the invalid fields without updating the object and updating the object when valid values are set. The hook needs to be passed the object to track changes on. It will return the following functions to the consumer: * onValueChange: callback to call when the value of an object's attribute is changed. It will handle the change and udpate the object. It will not track it, to allow side-effect of the change done by the form calling it. * canUndo: returns whether there are changes that can be undone * canRedo: returns whether there are changes that can be redone * undo: undo the last change, it will update the object * redo: redo the last change, it will update the object * hasInvalidFields: return whether there are invalid fields in the form * updateHistory: Track the current object state
Let the form use the `useHistoryTracking` hook to allow to track object changes history. As the `TransitRouting` object is a simple one, without ID, or DB representation, it is a good candidate to be the first to migrate to the new form structure, using hooks instead of inheritance for history tracking. Also add an UndoRedoButtons widget to allow to undo/redo changes in the form.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
@packages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts:
- Around line 72-79: The test 'should undo additional of an optional fields'
currently expects { ...initialData, optionalField: undefined } which incorrectly
retains the optionalField key; update the assertion to expect the original
initialData instead (use initialData) so that after tracker.record(...) and
tracker.undo() the call to tracker.current() is compared to the exact initial
state (initialData), ensuring the optional field is removed.
In
@packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsx:
- Around line 18-31: The TestObject class declaration ends with an unnecessary
semicolon after its closing brace; remove the trailing semicolon following the
TestObject class definition so the class ends with just the closing brace (i.e.,
update the TestObject declaration that extends BaseObject<TestAttributes> to
drop the final semicolon).
In @packages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsx:
- Line 92: The generic constraint on useHistoryTracker uses BaseObject<any>;
replace the any with a safer type like Record<string, unknown> so the signature
becomes useHistoryTracker<T extends BaseObject<Record<string, unknown>>>({
object }: { object: T }): WithHistoryTracker<T> — update the generic bound
wherever BaseObject<any> appears in this file to BaseObject<Record<string,
unknown>> to comply with typing guidelines.
In @packages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsx:
- Around line 34-55: Remove the redundant static React key props from the two
Button components in UndoRedoButtons (the Button with name="undo" and the Button
with name="redo"); since these sibling buttons are static and neither their
order nor count changes, delete key="undo" and key="redo" to clean up the JSX
while leaving the other props (title, name, color, disabled, icon, iconClass,
label, onClick) intact.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
packages/chaire-lib-common/src/utils/objects/HistoryTracker.tspackages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.tspackages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsxpackages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsxpackages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
**/*.{ts,tsx}: Use TypeScript strictly - avoidanytypes in all files
All geographic data must use GeoJSON format with coordinates as [longitude, latitude]
Document non-trivial functions and attributes using JSDoc or inline comments when short
Use parentheses for all arrow function parameters, even single parameters
Use @turf/turf and geojson libraries for geospatial calculations and GeoJSON type definitions
Frontend build process requires building frontend withyarn build:devoryarn build:prodbefore starting backend server
Files:
packages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsxpackages/chaire-lib-common/src/utils/objects/HistoryTracker.tspackages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsxpackages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsxpackages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts
**/*.{ts,tsx,js,jsx,json,scss,sass}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
Indent using 4 spaces throughout all code files
Files:
packages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsxpackages/chaire-lib-common/src/utils/objects/HistoryTracker.tspackages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsxpackages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsxpackages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts
**/*.{ts,tsx,js,jsx,json,scss,sass,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
No trailing spaces in any lines, including comments and documentation; replace lines with spaces only with blank lines
Files:
packages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsxpackages/chaire-lib-common/src/utils/objects/HistoryTracker.tspackages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsxpackages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsxpackages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts
**/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
All user-facing strings must use i18next internationalization with the
t()function
Files:
packages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsxpackages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsxpackages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
**/*.{ts,tsx,js,jsx,json,scss,sass,md}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
Code formatting must follow Prettier configuration from configs/ directory; run
yarn formatbefore committing
Files:
packages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsxpackages/chaire-lib-common/src/utils/objects/HistoryTracker.tspackages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsxpackages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsxpackages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
All code must be linted with
yarn lintand formatted withyarn formatbefore committing
Files:
packages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsxpackages/chaire-lib-common/src/utils/objects/HistoryTracker.tspackages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsxpackages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsxpackages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts
**/__tests__/**/*.{test,spec}.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
**/__tests__/**/*.{test,spec}.{ts,tsx,js}: Use parametric tests in Jest when more than one expect statement is used to simplify debugging of failed tests
Testing framework must use Jest for unit tests and Playwright for UI tests; test files located in tests/ directories
Files:
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts
**/transition-frontend/src/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
Frontend React components should be organized by feature in packages/transition-frontend/src/components/ with dedicated subdirectories for forms, dashboard, map components, etc.
Files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
**/transition-frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
Use Redux with Redux Thunk for state management in React components
Files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
**/__tests__/**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/project-rule.mdc)
Sequential/integration tests require separate test database setup with
yarn setup-test && yarn migrate-testbefore running
Files:
packages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/transition-frontend/src/**/*.{ts,tsx} : Use Redux with Redux Thunk for state management in React components
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/transition-frontend/src/**/*.{ts,tsx} : Use Redux with Redux Thunk for state management in React components
Applied to files:
packages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsxpackages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/transition-frontend/src/components/**/*.{ts,tsx} : Frontend React components should be organized by feature in packages/transition-frontend/src/components/ with dedicated subdirectories for forms, dashboard, map components, etc.
Applied to files:
packages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsxpackages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/*.{ts,tsx} : Document non-trivial functions and attributes using JSDoc or inline comments when short
Applied to files:
packages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/__tests__/**/*.{test,spec}.{ts,tsx,js} : Use parametric tests in Jest when more than one expect statement is used to simplify debugging of failed tests
Applied to files:
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/__tests__/**/*.{test,spec}.{ts,tsx,js} : Testing framework must use Jest for unit tests and Playwright for UI tests; test files located in __tests__/ directories
Applied to files:
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts
📚 Learning: 2025-12-17T22:46:52.336Z
Learnt from: tahini
Repo: chairemobilite/transition PR: 1666
File: packages/chaire-lib-frontend/package.json:60-60
Timestamp: 2025-12-17T22:46:52.336Z
Learning: In packages/chaire-lib-frontend/package.json, the `testing-library/dom` devDependency is required for tests to pass even though it's not explicitly imported. It was previously provided as a transitive dependency through `react-select-event`, but after removing `react-select-event`, it must be explicitly declared.
Applied to files:
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/__tests__/**/*.test.ts : Sequential/integration tests require separate test database setup with `yarn setup-test && yarn migrate-test` before running
Applied to files:
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsxpackages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/transition-frontend/src/components/routers/TransitionRouter.tsx : Frontend routing must be defined in packages/transition-frontend/src/components/routers/TransitionRouter.tsx
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/transition-backend/src/services/transitRouting/**/*.ts : Transit routing calculations must be implemented using the service in packages/transition-backend/src/services/transitRouting/
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Transition is a full-stack TypeScript/React application for transit planning and simulation; always consider the complete architecture of modeling, simulating, and planning features
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/transition-frontend/src/app-transition.tsx : Frontend application entry point is packages/transition-frontend/src/app-transition.tsx
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.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/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/src/utils/processManagers/**/*.ts : Services using external routing engines must configure OSRM for road-based modes (walking, cycling, driving, bus) and trRouting for transit routing
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/transition-backend/src/services/gtfsImport/**/*.ts : GTFS import functionality must be implemented following the workflow in packages/transition-backend/src/services/gtfsImport/
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Applies to **/src/models/db/Transit*.ts : Transit objects follow a hierarchy: Agency > Lines > Services > Paths > Nodes; maintain this structure when designing data models
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-18T19:37:07.564Z
Learnt from: greenscientist
Repo: chairemobilite/transition PR: 1588
File: packages/transition-backend/src/services/transitRouting/TrRoutingBatchManager.ts:11-23
Timestamp: 2025-11-18T19:37:07.564Z
Learning: In the chairemobilite/transition repository TypeScript code, greenscientist prefers that reviews highlight when interfaces could be replaced with type aliases for stylistic consistency, especially when the structure is just a data shape.
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-25T21:28:28.491Z
Learnt from: kaligrafy
Repo: chairemobilite/transition PR: 1604
File: packages/transition-backend/src/services/weighting/NodeWeightCalculator.ts:74-79
Timestamp: 2025-11-25T21:28:28.491Z
Learning: In packages/transition-backend/src/services/weighting/NodeWeightCalculator.ts, the maxBirdDistanceMeters, maxNetworkDistanceMeters, and maxTravelTimeSeconds parameters should not accept 0 or negative values, as they are semantically invalid for distance and time thresholds. Explicit validation should be preferred over relying on falsy coercion.
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-09-25T17:08:56.828Z
Learnt from: greenscientist
Repo: chairemobilite/transition PR: 1494
File: packages/transition-backend/src/tasks/TransitionWorkerPool.ts:105-107
Timestamp: 2025-09-25T17:08:56.828Z
Learning: In packages/transition-backend/src/tasks/TransitionWorkerPool.ts, greenscientist deferred changing the wrapBatchAccessMap input file validation from `hasInputFile()` to `getFilePath('input')` (actual file existence check), indicating it will be addressed in an upcoming file access refactor.
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-09-25T17:09:26.332Z
Learnt from: greenscientist
Repo: chairemobilite/transition PR: 1494
File: packages/transition-backend/src/tasks/TransitionWorkerPool.ts:85-91
Timestamp: 2025-09-25T17:09:26.332Z
Learning: In packages/transition-backend/src/tasks/TransitionWorkerPool.ts, greenscientist deferred improving input file validation to check actual file existence on disk (using getFilePath('input') instead of just hasInputFile() metadata check), indicating it should be addressed when file access is refactored soon.
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Ensure trRouting service is running and scenario is loaded before attempting transit routing calculations
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
📚 Learning: 2025-11-24T18:37:56.827Z
Learnt from: CR
Repo: chairemobilite/transition PR: 0
File: .cursor/rules/project-rule.mdc:0-0
Timestamp: 2025-11-24T18:37:56.827Z
Learning: Scenarios represent complete transit network configurations; always ensure scenario consistency when modifying transit objects
Applied to files:
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx
🧬 Code graph analysis (4)
packages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsx (1)
packages/chaire-lib-frontend/src/components/input/Button.tsx (1)
Button(38-138)
packages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsx (2)
packages/chaire-lib-common/src/utils/objects/HistoryTracker.ts (1)
HistoryTracker(13-111)packages/chaire-lib-common/src/utils/objects/ObjectWithHistory.ts (4)
undo(57-61)canUndo(49-51)redo(63-67)canRedo(53-55)
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsx (1)
packages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsx (1)
useHistoryTracker(92-237)
packages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts (1)
packages/chaire-lib-common/src/utils/objects/HistoryTracker.ts (1)
HistoryTracker(13-111)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: code-lint
- GitHub Check: build-and-test (24.x)
- GitHub Check: test-sequential (24.x)
- GitHub Check: pr-build-check
- GitHub Check: build-and-run-UI-tests (24.x)
🔇 Additional comments (8)
packages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts (1)
1-162: Good test coverage for HistoryTracker.The test suite covers initialization, undo/redo boundaries, optional fields, reset behavior, history truncation, change detection, and immutability. This aligns well with the HistoryTracker public API.
packages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsx (1)
1-60: Clean and well-documented component.Good use of i18n, clear prop types with JSDoc, and proper disabled state handling.
packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsx (1)
1-501: Thorough test coverage for useHistoryTracker hook.Tests cover field updates, invalid values, undo/redo operations, atomic multi-field changes, invalid field tracking, and behavior across re-renders with varying object identities. Well structured.
packages/transition-frontend/src/components/forms/transitRouting/TransitRoutingForm.tsx (2)
46-75: Good integration of useHistoryTracker hook.Clean destructuring of hook return values and proper setup of the history-tracked form state.
469-469: Nice addition of undo/redo UI controls.The UndoRedoButtons integration provides a clean interface for history navigation.
packages/chaire-lib-common/src/utils/objects/HistoryTracker.ts (1)
1-111: Well-implemented history tracking class.Clean API with proper immutability through deep cloning. JSDoc documentation is thorough. The bounded history mechanism with
maxCountis a nice touch to prevent unbounded memory growth.packages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsx (2)
1-237: Solid hook implementation.Good separation of concerns with refs for immediate access and state for rendering. The history tracking integration is clean, and the API is well-documented.
197-197: Remove this concern - constructor pattern is type-safe and consistently followed.All BaseObject subclasses in the codebase (TransitRouting, TestObject) follow the same constructor signature accepting
Partial<T>, which matches the type cast used in lines 197 and 220. TypeScript generics enforce this consistency through the constraintT extends BaseObject<any>. The pattern is not fragile.However, note: The code uses
anytype, which violates the guideline to avoidanytypes in all TypeScript files.Likely an incorrect or invalid review comment.
| test('should undo additional of an optional fields', () => { | ||
| // Update an optional field | ||
| const newDataWithOptionalField: TestType = { ...initialData, optionalField: 'test' }; | ||
| tracker.record(newDataWithOptionalField); | ||
| tracker.undo(); | ||
|
|
||
| expect(tracker.current()).toEqual({ ...initialData, optionalField: undefined }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, locate the test file
find . -name "HistoryTracker.test.ts" -type fRepository: chairemobilite/transition
Length of output: 148
🏁 Script executed:
#!/bin/bash
# Read the test file to understand the structure
cat -n packages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts | head -100Repository: chairemobilite/transition
Length of output: 4265
🏁 Script executed:
#!/bin/bash
# Find and examine the HistoryTracker implementation
find . -name "HistoryTracker.ts" -type f | grep -v testRepository: chairemobilite/transition
Length of output: 133
🏁 Script executed:
#!/bin/bash
# Read the HistoryTracker implementation
cat -n packages/chaire-lib-common/src/utils/objects/HistoryTracker.tsRepository: chairemobilite/transition
Length of output: 4649
Fix the expected value in the optional field undo test.
Line 78 expects { ...initialData, optionalField: undefined }, but this explicitly includes an optionalField key that doesn't exist in initialData. Undoing should restore the exact initial state { id: 1, value: 10 } without the optionalField key. Change the expectation to initialData.
🤖 Prompt for AI Agents
In
@packages/chaire-lib-common/src/utils/objects/__tests__/HistoryTracker.test.ts
around lines 72 - 79, The test 'should undo additional of an optional fields'
currently expects { ...initialData, optionalField: undefined } which incorrectly
retains the optionalField key; update the assertion to expect the original
initialData instead (use initialData) so that after tracker.record(...) and
tracker.undo() the call to tracker.current() is compared to the exact initial
state (initialData), ensuring the optional field is removed.
| class TestObject extends BaseObject<TestAttributes> { | ||
| protected _validate(): [boolean, string[]] { | ||
| return [true, []]; | ||
| } | ||
| protected _prepareAttributes(attributes: Partial<TestAttributes>): TestAttributes { | ||
| return { | ||
| field1: '', | ||
| ...attributes | ||
| } | ||
| } | ||
| get id() { | ||
| return this.attributes.id; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Unnecessary semicolon after class definition.
Line 31 has a semicolon after the closing brace of the class, which is valid but unnecessary in TypeScript/JavaScript.
🤖 Prompt for AI Agents
In
@packages/chaire-lib-frontend/src/components/forms/__tests__/useHistoryTracker.test.tsx
around lines 18 - 31, The TestObject class declaration ends with an unnecessary
semicolon after its closing brace; remove the trailing semicolon following the
TestObject class definition so the class ends with just the closing brace (i.e.,
update the TestObject declaration that extends BaseObject<TestAttributes> to
drop the final semicolon).
| * @param object The object to track changes on | ||
| * @returns | ||
| */ | ||
| export const useHistoryTracker = <T extends BaseObject<any>>({ object }: { object: T }): WithHistoryTracker<T> => { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid any type in generic constraint.
Per coding guidelines, avoid any types. Consider using BaseObject<Record<string, unknown>> instead.
♻️ Suggested fix
-export const useHistoryTracker = <T extends BaseObject<any>>({ object }: { object: T }): WithHistoryTracker<T> => {
+export const useHistoryTracker = <T extends BaseObject<Record<string, unknown>>>({ object }: { object: T }): WithHistoryTracker<T> => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const useHistoryTracker = <T extends BaseObject<any>>({ object }: { object: T }): WithHistoryTracker<T> => { | |
| export const useHistoryTracker = <T extends BaseObject<Record<string, unknown>>>({ object }: { object: T }): WithHistoryTracker<T> => { |
🤖 Prompt for AI Agents
In @packages/chaire-lib-frontend/src/components/forms/useHistoryTracker.tsx at
line 92, The generic constraint on useHistoryTracker uses BaseObject<any>;
replace the any with a safer type like Record<string, unknown> so the signature
becomes useHistoryTracker<T extends BaseObject<Record<string, unknown>>>({
object }: { object: T }): WithHistoryTracker<T> — update the generic bound
wherever BaseObject<any> appears in this file to BaseObject<Record<string,
unknown>> to comply with typing guidelines.
| <Button | ||
| title={t('main:Undo')} | ||
| name="undo" | ||
| key="undo" | ||
| color="grey" | ||
| disabled={!props.canUndo()} | ||
| icon={faUndoAlt} | ||
| iconClass="_icon-alone" | ||
| label="" | ||
| onClick={props.onUndo} | ||
| /> | ||
| <Button | ||
| title={t('main:Redo')} | ||
| name="redo" | ||
| key="redo" | ||
| color="grey" | ||
| disabled={!props.canRedo()} | ||
| icon={faRedoAlt} | ||
| iconClass="_icon-alone" | ||
| label="" | ||
| onClick={props.onRedo} | ||
| /> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: key props are unnecessary on static sibling elements.
The key="undo" and key="redo" props aren't needed since these are static siblings that don't change order or count. They're harmless but redundant.
🤖 Prompt for AI Agents
In @packages/chaire-lib-frontend/src/components/pageParts/UndoRedoButtons.tsx
around lines 34 - 55, Remove the redundant static React key props from the two
Button components in UndoRedoButtons (the Button with name="undo" and the Button
with name="redo"); since these sibling buttons are static and neither their
order nor count changes, delete key="undo" and key="redo" to clean up the JSX
while leaving the other props (title, name, color, disabled, icon, iconClass,
label, onClick) intact.
2 participants
This is a first step towards removing the
ObjectWithHistoryfrom the class hierarchy. Tracking history is not required for most use cases except when editing from the UI. Also, the current way of tracking history was to save history after each field set, resulting in non-atomic updates (see #9), requiring multiple undo/redo to really go back to previous state.We add a
HistoryTrackerclass which saves an object's attributes history. This tracker is used in theuseHistoryTrackerhook that forms can use to manage value changes (a step toward #48). The form decide when it should save the history, after all attribute updates have been done.The
TransitRoutingFormhas been updated to use this new approach, as it does not concern a saveable object, making a good first candidate to implement the new approach. This actually adds the undo/redo functionality to the TransitRouting form, that did not previously exist.Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.