feat(tabular-modification): inline cell edit and row manipulation#3948
Open
flomillot wants to merge 6 commits into
Open
feat(tabular-modification): inline cell edit and row manipulation#3948flomillot wants to merge 6 commits into
flomillot wants to merge 6 commits into
Conversation
The tabular modification/creation dialog used to be fed exclusively by a CSV import: rows were displayed read-only and any correction required editing the CSV outside the app and re-importing it. Replace the read-only CustomAGGrid by CustomAgGridTable, which brings inline edit, multi-row selection + delete, add row, drag handle and move up/down via its bottom-right buttons. The existing CSV import flow on top of the dialog is preserved untouched; CSV-parsed rows are loaded into the grid imperatively through a forwarded ref to the internal useFieldArray. Details: - Inject AG_GRID_ROW_UUID on each row when loading from CSV (handleComplete) and when restoring an existing modification via editData (initTabularCreationData / initTabularModificationData). CustomAgGridTable uses this UUID for getRowId, so without it row tracking degenerates to "undefined === undefined", which silently collapses the field array. - Make columns editable with the right cell editor per field type: agSelectCellEditor for BOOLEAN and ENUM, NumericEditor for NUMBER, default text editor otherwise. Properties columns are editable as text. - Mark the pinned EQUIPMENT_ID column as the row drag handle (rowDrag: true), now that commons-ui no longer hardcodes rowDragEntireRow. - Replace every setValue(MODIFICATIONS_TABLE, ...) by replace() via the table ref to keep the underlying useFieldArray in sync (a second useFieldArray on the same name is forbidden by RHF). - Drop the top-of-dialog ErrorInput; BottomRightButtons already renders one right under the grid. - Add the agGridBackground token to the gridstudy theme (light and dark) so CustomAgGridTable's styles can read theme.agGridBackground.color. Requires commons-ui change forwarding useFieldArray via ref and making row drag opt-in (gridsuite/commons-ui#1133). Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>
📝 WalkthroughWalkthroughThis PR refactors the tabular network modifications grid from form-state-driven rendering to imperative ref-based updates. It adds UUID row identifiers during initialization, updates MUI theme styling for grid backgrounds, and switches grid rendering from CustomAGGrid to CustomAgGridTable with specialized column editors and ref-backed data updates. ChangesTabular Grid Refactoring
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
flomillot
changed the title
…olDef DefaultCellRenderer was repeated for every typed branch (BOOLEAN, NUMBER, ENUM, default). Move it once into defaultColDef and drop the per-case assignments. BOOLEAN now falls through to the default text editor instead of agSelectCellEditor + BooleanNullableCellRenderer; remove the dead cellEditorParams and the now-unused BooleanNullableCellRenderer import. Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>
Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>
… selection props Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>
…proved layout consistency Signed-off-by: Florent MILLOT <75525996+flomillot@users.noreply.github.com>
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/dialogs/network-modifications/tabular/tabular-dialog.tsx`:
- Line 121: The UI-only FieldConstants.AG_GRID_ROW_UUID is being added to row
objects for grid state but is leaking into API payloads; update the submission
path to strip this key before backend conversion by adding an explicit guard
that ignores FieldConstants.AG_GRID_ROW_UUID: either add a top-level filter in
convertCreationFieldFromFrontToBack to return undefined/skip when the field key
=== FieldConstants.AG_GRID_ROW_UUID, and/or add a pre-iteration filter in the
modification transformation strategy (the function that iterates row keys to
build modification payloads) so it never processes that key; ensure both
creation and modification flows (the code that calls
convertCreationFieldFromFrontToBack and the modification transformer) exclude
AG_GRID_ROW_UUID before assembling the final payload.
🪄 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: CHILL
Plan: Pro
Run ID: 755a86b8-0269-472d-87b8-7139176f1f44
📒 Files selected for processing (4)
src/components/app-wrapper.jsxsrc/components/dialogs/network-modifications/tabular/tabular-dialog.tsxsrc/components/dialogs/network-modifications/tabular/tabular-form.tsxsrc/module-mui.d.ts
| } | ||
| } | ||
| modification = addPropertiesFromBack(modification, modif?.[TABULAR_PROPERTIES]); | ||
| modification[FieldConstants.AG_GRID_ROW_UUID] = uuid4(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 'AG_GRID_ROW_UUID|Object\.keys\(row\)|convertCreationFieldFromFrontToBack|transformModificationsTable' \
src/components/dialogs/network-modifications/tabularRepository: gridsuite/gridstudy-app
Length of output: 10301
🏁 Script executed:
cat -n src/components/dialogs/network-modifications/tabular/tabular-creation-utils.ts | sed -n '188,210p'Repository: gridsuite/gridstudy-app
Length of output: 583
🏁 Script executed:
cat -n src/components/dialogs/network-modifications/tabular/tabular-modification-utils.ts | sed -n '486,510p'Repository: gridsuite/gridstudy-app
Length of output: 463
🏁 Script executed:
rg -n -A30 'transformRowToBackEndModification' src/components/dialogs/network-modifications/tabular/tabular-modification-utils.ts | head -60Repository: gridsuite/gridstudy-app
Length of output: 1455
🏁 Script executed:
rg -n 'AG_GRID_ROW_UUID' src/components/dialogs/network-modifications/tabular/tabular-modification-utils.tsRepository: gridsuite/gridstudy-app
Length of output: 49
🏁 Script executed:
rg -n 'AG_GRID_ROW_UUID' src/components/dialogs/network-modifications/tabular/tabular-creation-utils.tsRepository: gridsuite/gridstudy-app
Length of output: 49
Remove AG_GRID_ROW_UUID before submitting to backend.
Lines 121 and 139–140 add FieldConstants.AG_GRID_ROW_UUID to row objects for UI state tracking. However, this field propagates to the API payload:
- Creation submissions (lines 195–197) iterate all row keys without filtering, passing each through
convertCreationFieldFromFrontToBack, which has no guard for the UUID field. - Modification submissions (line 166) delegate to transformation strategies that iterate all keys (line 444) without filtering.
Add an explicit check in convertCreationFieldFromFrontToBack or in the modification transformation strategy to exclude FieldConstants.AG_GRID_ROW_UUID before the backend payload is assembled.
Also applies to: 137–140
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/dialogs/network-modifications/tabular/tabular-dialog.tsx` at
line 121, The UI-only FieldConstants.AG_GRID_ROW_UUID is being added to row
objects for grid state but is leaking into API payloads; update the submission
path to strip this key before backend conversion by adding an explicit guard
that ignores FieldConstants.AG_GRID_ROW_UUID: either add a top-level filter in
convertCreationFieldFromFrontToBack to return undefined/skip when the field key
=== FieldConstants.AG_GRID_ROW_UUID, and/or add a pre-iteration filter in the
modification transformation strategy (the function that iterates row keys to
build modification payloads) so it never processes that key; ensure both
creation and modification flows (the code that calls
convertCreationFieldFromFrontToBack and the modification transformer) exclude
AG_GRID_ROW_UUID before assembling the final payload.
Comment on lines
+302
to
+306
| const rowsWithUuid = results.data.map((row) => ({ | ||
| ...row, | ||
| [FieldConstants.AG_GRID_ROW_UUID]: uuid4(), | ||
| })); | ||
| tableRef.current?.replace(rowsWithUuid); |
There was a problem hiding this comment.
Inline edits/additions bypass the only real validation path.
The grid is editable and can create new rows now, but the required/type checks still only run during CSV parsing. That means users can add blank rows or change a cell to an invalid value and still save because errors never refresh; conversely, fixing a CSV error inline can leave the form blocked. Please re-run row validation on grid mutations or validate the current table again in onSave.
Also applies to: 411-440, 444-453, 545-559
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Until now, the tabular modification/creation dialog only supported read-only CSV import: any correction required editing the CSV outside the app and re-importing it. This PR adds inline cell editing and row deletion directly in the UI.
Key changes
CustomAGGrid(read-only) →CustomAgGridTable(commons-ui): inline editing, multi-row selection + deletion, row add, drag handle, move up/down.useFieldArray— RHF forbids a seconduseFieldArrayon the same name.agSelectCellEditorfor ENUM (values fromfield.options),NumericEditorfor NUMBER, default text editor otherwise (including BOOLEAN).property_*columns are text-editable.EQUIPMENT_ID(pinned) column receivesrowDrag: true— AG Grid's default drag handle, following the removal ofrowDragEntireRowfrom commons-ui.AG_GRID_ROW_UUIDis injected per row at all three entry points: CSV parse (handleComplete), editing an existing modification (initTabularCreationData/initTabularModificationData). Required for the grid'sgetRowIdto be stable.agGridBackgroundtheme token (light + dark) consumed byCustomAgGridTablestyles.ErrorInput(duplicate:BottomRightButtonsalready renders one under the grid).Known limitations / follow-up
Typed validation (required, requiredIf, BOOLEAN/NUMBER/ENUM) is currently only re-executed on CSV parse. After inline edit / add / delete, the alert does not refresh and submit-side validation remains minimal (
yup.array().min(1)only). Follow-up is planned.Dependency
Requires gridsuite/commons-ui#1133 to be merged and released.