Conversation
Signed-off-by: SOUISSI Maissa (Externe) <souissimai@gm0winl878.bureau.si.interne>
There was a problem hiding this comment.
The tooltip above the description used to be on the right :
It allowed the user to go from top to bottom, watching each description without being bothered by the tooltip in its way.
Now :
It displays under the icon and gets in the way. This is annoying but also a regression : Stephane specifically asked for the tooltip on the right. (and it also looks nicer with the arrow)
.../graph/menus/network-modifications/network-modification-table/renderers/description-cell.tsx
Outdated
Show resolved
Hide resolved
...onents/graph/menus/network-modifications/network-modification-table/row/modification-row.tsx
Outdated
Show resolved
Hide resolved
...onents/graph/menus/network-modifications/network-modification-table/row/modification-row.tsx
Outdated
Show resolved
Hide resolved
| {...(cell.column.id === BASE_MODIFICATION_TABLE_COLUMNS.DRAG_HANDLE.id | ||
| ? provided.dragHandleProps | ||
| : undefined)} |
There was a problem hiding this comment.
If you handle this case in the previous if (isDragHandle) {, what is the point of this ?
There was a problem hiding this comment.
then we remove all onClick prop usage ?
There was a problem hiding this comment.
No I only talks about :
{...(cell.column.id === BASE_MODIFICATION_TABLE_COLUMNS.DRAG_HANDLE.id
? provided.dragHandleProps
: undefined)}
The specific case when
cell.column.id === BASE_MODIFICATION_TABLE_COLUMNS.DRAG_HANDLE.id
is already handled in the previous if (isDragHandle) { which then returns :
<TableCell
key={cell.id}
sx={createCellStyle(cell, AUTO_EXTENSIBLE_COLUMNS.includes(cell.column.id))}
>
<Tooltip title={<FormattedMessage id={'moveModification'} />} arrow>
<span {...provided.dragHandleProps}>{cellContent}</span>
</Tooltip>
</TableCell>
Therefore I think that case will never happen and this if can be removed. But onClick remains, it is useful to trigger the modification dialog when the name is clicked..
📝 WalkthroughWalkthroughUpdated UI text and tooltip rendering for network modification components, added context-specific internationalized messages, adjusted per-column table-cell rendering (drag-handle and select columns) to wrap content in tooltips, and added corresponding English and French translation keys. Changes
Sequence Diagram(s)mermaid Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/components/graph/menus/network-modifications/network-modification-table/renderers/description-cell.tsx`:
- Line 63: The Tooltip currently uses the nullish coalescing operator with
description (Tooltip title={description ?? <FormattedMessage
id={'addDescription'} />}), which fails when description is an empty string;
change the check to a non-empty string test instead (e.g., use
description?.trim() or description && description.length) so that when
description is empty the fallback FormattedMessage ('addDescription') is shown;
update the Tooltip title expression and ensure it still references the same
description variable and the existing FormattedMessage id.
In
`@src/components/graph/menus/network-modifications/network-modification-table/row/modification-row.tsx`:
- Around line 50-55: Remove the temporary debug log inside the cell render loop:
eliminate the console.log(cellContent, 'pppp') that runs in the mapping over
row.getVisibleCells() (used with flexRender(cell.column.columnDef.cell,
cell.getContext())). Locate the map callback where row.getVisibleCells().map is
used and delete that console.log; if needed keep a conditional debug only behind
a dev-only flag or debugger statement rather than logging on every cell render
(especially for columns referenced via
BASE_MODIFICATION_TABLE_COLUMNS.DRAG_HANDLE and .SELECT).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5ffb7ceb-8106-482f-91d5-60ea31708a74
📒 Files selected for processing (7)
src/components/graph/menus/network-modifications/network-modification-node-editor.tsxsrc/components/graph/menus/network-modifications/network-modification-table/columns-definition.tsxsrc/components/graph/menus/network-modifications/network-modification-table/renderers/description-cell.tsxsrc/components/graph/menus/network-modifications/network-modification-table/renderers/switch-cell.tsxsrc/components/graph/menus/network-modifications/network-modification-table/row/modification-row.tsxsrc/translations/en.jsonsrc/translations/fr.json
.../graph/menus/network-modifications/network-modification-table/renderers/description-cell.tsx
Outdated
Show resolved
Hide resolved
...onents/graph/menus/network-modifications/network-modification-table/row/modification-row.tsx
Show resolved
Hide resolved
| )} | ||
| <Tooltip title={description} arrow placement="right"> | ||
| <Tooltip title={description ?? <FormattedMessage id={'addDescription'} />}> | ||
| <IconButton |
There was a problem hiding this comment.
add a span wrapping the IconButton here which could be disabled ad proc a well known console warning
Co-authored-by: Mathieu Deharbe <148252167+Mathieu-Deharbe@users.noreply.github.com>
|
There was a problem hiding this comment.
🧹 Nitpick comments (5)
src/translations/en.json (3)
1761-1761: Trailing whitespace after closing brace.The file ends with a trailing space after the closing brace
}. This is typically flagged by linters.🔧 Suggested fix
-} +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/en.json` at line 1761, The file ends with a trailing space after the final closing brace "}" in en.json; remove the trailing whitespace so the file ends exactly with "}" (no extra spaces or newline characters after it) to satisfy the linter.
11-11: Trailing whitespace after comma.Line 11 has a trailing space after the comma which is typically flagged by linters and formatters.
🔧 Suggested fix
- "filter": "Filter", + "filter": "Filter",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/en.json` at line 11, The JSON entry for the "filter" key has a trailing whitespace after the comma; remove the extra space so the line reads `"filter": "Filter",` without any trailing space after the comma (locate the `"filter"` key in the translations file and trim the trailing whitespace).
1752-1760: Inconsistent JSON formatting: some keys have extra space before the colon.The keys
downloadNetworkModifications,addDescription,activateModification, anddeactivateModificationuse:(space before colon), while the rest of the file and other new keys use:(no space before colon). This inconsistency may trigger linter warnings.🔧 Suggested fix for consistent formatting
- "downloadNetworkModifications" : "Download modifications", + "downloadNetworkModifications": "Download modifications", "moveToTrash": "Move to trash", "visualizedRootNetwork": "Visualized root network : {tag}", - "addDescription" : "Add description", - "activateModification" : "Activate network modification", - "deactivateModification" : "Deactivate network modification", + "addDescription": "Add description", + "activateModification": "Activate network modification", + "deactivateModification": "Deactivate network modification", "deselectModification": "Deselect modification", "selectModification": "Select modification", "moveModification": "Move modification"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/en.json` around lines 1752 - 1760, The JSON keys downloadNetworkModifications, addDescription, activateModification, and deactivateModification have an extra space before the colon causing inconsistent formatting; open the translations JSON and remove the extra space so each key uses "key": "value" (no space before the colon) to match the rest of the file and satisfy the linter — update the entries for downloadNetworkModifications, addDescription, activateModification, and deactivateModification accordingly and verify the file still parses as valid JSON.src/translations/fr.json (2)
1745-1753: Inconsistent JSON formatting.Several entries have inconsistent spacing:
- Lines 1745, 1748, 1749, 1750: Space before colon (
:instead of:)- Line 1751: Space before comma (
,instead of,)- Line 1753: Trailing space at end of line
The rest of the file uses
":without a preceding space. These should be aligned for consistency.🧹 Suggested fix
- "downloadNetworkModifications" : "Télécharger les modifications", + "downloadNetworkModifications": "Télécharger les modifications", "moveToTrash": "Mettre à la corbeille", "visualizedRootNetwork": "Réseau racine visualisé : {tag}", - "addDescription" : "Ajouter une description", - "activateModification" : "Activer la modification réseau", - "deactivateModification" : "Désactiver la modification réseau", - "deselectModification": "Désélectionner la modification" , + "addDescription": "Ajouter une description", + "activateModification": "Activer la modification réseau", + "deactivateModification": "Désactiver la modification réseau", + "deselectModification": "Désélectionner la modification", "selectModification": "Sélectionner la modification", - "moveModification": "Déplacer la modification" + "moveModification": "Déplacer la modification" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/fr.json` around lines 1745 - 1753, The JSON keys "downloadNetworkModifications", "addDescription", "activateModification", and "deactivateModification" have an extra space before the colon, "deselectModification" has a space before the comma, and "moveModification" has a trailing space; update those entries to remove the spaces so they follow the project's standard key formatting (e.g., change `"downloadNetworkModifications" :` to `"downloadNetworkModifications":`, remove the stray space before `,` in `"deselectModification": "Désélectionner la modification" ,`, and trim the trailing space in `"moveModification": "Déplacer la modification" `) while leaving the translation text unchanged.
11-11: Trailing whitespace introduced.There's a trailing space after the comma on this line. This appears unintentional and is inconsistent with the rest of the file.
🧹 Suggested fix
- "filter": "Filtre", + "filter": "Filtre",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/fr.json` at line 11, The JSON entry for the translation key "filter" contains an unintended trailing space after the comma; open the fr.json entry for the "filter" key and remove the extra whitespace so the line reads exactly "filter": "Filtre", matching the file's formatting conventions and avoiding trailing whitespace.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/translations/en.json`:
- Line 1761: The file ends with a trailing space after the final closing brace
"}" in en.json; remove the trailing whitespace so the file ends exactly with "}"
(no extra spaces or newline characters after it) to satisfy the linter.
- Line 11: The JSON entry for the "filter" key has a trailing whitespace after
the comma; remove the extra space so the line reads `"filter": "Filter",`
without any trailing space after the comma (locate the `"filter"` key in the
translations file and trim the trailing whitespace).
- Around line 1752-1760: The JSON keys downloadNetworkModifications,
addDescription, activateModification, and deactivateModification have an extra
space before the colon causing inconsistent formatting; open the translations
JSON and remove the extra space so each key uses "key": "value" (no space before
the colon) to match the rest of the file and satisfy the linter — update the
entries for downloadNetworkModifications, addDescription, activateModification,
and deactivateModification accordingly and verify the file still parses as valid
JSON.
In `@src/translations/fr.json`:
- Around line 1745-1753: The JSON keys "downloadNetworkModifications",
"addDescription", "activateModification", and "deactivateModification" have an
extra space before the colon, "deselectModification" has a space before the
comma, and "moveModification" has a trailing space; update those entries to
remove the spaces so they follow the project's standard key formatting (e.g.,
change `"downloadNetworkModifications" :` to `"downloadNetworkModifications":`,
remove the stray space before `,` in `"deselectModification": "Désélectionner la
modification" ,`, and trim the trailing space in `"moveModification": "Déplacer
la modification" `) while leaving the translation text unchanged.
- Line 11: The JSON entry for the translation key "filter" contains an
unintended trailing space after the comma; open the fr.json entry for the
"filter" key and remove the extra whitespace so the line reads exactly "filter":
"Filtre", matching the file's formatting conventions and avoiding trailing
whitespace.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3e737a9f-6777-4311-95db-9afaf890a4ac
📒 Files selected for processing (4)
src/components/graph/menus/network-modifications/network-modification-table/renderers/description-cell.tsxsrc/components/graph/menus/network-modifications/network-modification-table/row/modification-row.tsxsrc/translations/en.jsonsrc/translations/fr.json
✅ Files skipped from review due to trivial changes (1)
- src/components/graph/menus/network-modifications/network-modification-table/renderers/description-cell.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/graph/menus/network-modifications/network-modification-table/row/modification-row.tsx





PR Summary