Fix: responsive grids - #311
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📝 WalkthroughWalkthroughThe PR rebuilds MUI table components under ChangesMUI table restructuring
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MuiTable
participant TableShell
participant SortableHeaderContent
participant TableCellContent
participant RowActionsMenu
participant createDeleteHandler
MuiTable->>TableShell: render table content and pagination
MuiTable->>SortableHeaderContent: provide sort state and callback
MuiTable->>TableCellContent: render row cell values
MuiTable->>RowActionsMenu: provide responsive row actions
RowActionsMenu->>createDeleteHandler: invoke delete action
createDeleteHandler-->>MuiTable: confirm item deletion
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (1)
src/components/mui/tables/mui-table/index.js (1)
203-203: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCoerce
disabledto a boolean.
options.disableProp && row[options.disableProp]can evaluate tonull,undefined, a string, or a number.IconButtonexpects a boolean fordisabled.getRowActionsalready applies!!(...)for the same conditions, so the menu path and the icon path can disagree in typing. Apply!!at each icon button for consistency.♻️ Proposed change
- disabled={options.disableProp && row[options.disableProp]} + disabled={!!(options.disableProp && row[options.disableProp])}Also applies to: 234-238, 268-268, 293-293
🤖 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/mui/tables/mui-table/index.js` at line 203, Update each IconButton disabled prop in the table row-action rendering, including the instances near the current expression and the additional highlighted locations, to coerce the disable condition with !!. Match the boolean coercion already used by getRowActions so every icon path passes a boolean to IconButton.
🤖 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/mui/tables/components/create-delete-handler.js`:
- Around line 19-20: Update the dialog options in create-delete-handler to use
the showConfirmDialog API field iconType instead of type, preserving the
"warning" value so the delete confirmation displays its warning state.
- Around line 4-10: Update createDeleteHandler to accept a getId callback and
use it when invoking onDelete instead of always reading item.id. In
sortable-table, pass a callback that returns item[idKey], preserving the
configured row identifier for deletion while retaining existing behavior for the
default idKey.
In `@src/components/mui/tables/components/row-actions-menu.js`:
- Around line 28-34: Add a localized aria-label to the icon-only IconButton that
opens the row actions menu, using the component’s existing localization
mechanism and an existing translation key where available. Preserve the current
click behavior and data-testid.
In `@src/components/mui/tables/components/sortable-header-content.js`:
- Around line 15-17: Update the onClick handler in the sortable header content
to pass ascending order when col.columnKey differs from sortCol, and only negate
sortDir when the clicked column is already active. Preserve the existing column
key passed to onSort.
In `@src/components/mui/tables/components/styles.module.less`:
- Around line 7-12: Center the absolutely positioned border pseudo-element by
updating its positioning declarations: replace the top-zero alignment with top:
50% and apply a vertical translate matching half its 60% height. Remove the
ineffective align-self declaration while preserving the existing bottom, left,
border, and height styling.
In `@src/components/mui/tables/components/use-scroll-fade.js`:
- Around line 15-23: Update the React.useEffect around updateFades to observe
container and table dimension changes with a ResizeObserver, invoking
updateFades when either resizes. Observe the relevant container and table
elements, and disconnect the observer during cleanup alongside removing the
scroll listener.
In `@src/components/mui/tables/editable-table/index.js`:
- Around line 251-263: Update the editable-cell TableCell rendering around
isEditable, adding tabIndex only for editable cells and handling Enter or Space
keyboard activation by invoking handleCellClick with the row and column key.
Ignore keyboard events originating from the nested TextField, while preserving
non-editable cell behavior.
In `@src/components/mui/tables/mui-table/index.js`:
- Around line 115-119: Update the select action in the table component to use
the existing T.translate mechanism for the "View" label, including both the menu
entry and its tooltip. Add or reuse the appropriate translation key so both
visible labels follow the current locale while preserving the existing onSelect
behavior.
- Line 180: Update the className construction in the table cell rendering to
omit falsy or undefined values instead of interpolating them as strings.
Preserve both the conditional dotted-border class from col.dottedBorder and
col.className when present, while ensuring the rendered class list contains only
valid class names.
In `@src/components/mui/tables/sortable-table/index.js`:
- Around line 77-80: Use the configured idKey consistently for item identity in
the sortable table. Update the reorder logic around movedItemId and the
createDeleteHandler helper to derive IDs through a getId callback, then pass
(item) => item[idKey] when creating the delete handler so onReorder and onDelete
receive the configured item ID.
- Around line 182-217: Add localized aria-label values to the edit, delete, and
reorder IconButton elements in the table row actions, using the existing
localization mechanism and clear action-specific labels while preserving their
current click and drag behavior.
- Around line 66-75: Update the reorder logic around reordered and
updateOrderKey so assigning the new order does not mutate item objects supplied
through data. Create new item objects when applying updateOrderKey, while
preserving unchanged item references or values as appropriate, then pass the
resulting reordered collection to onReorder.
---
Nitpick comments:
In `@src/components/mui/tables/mui-table/index.js`:
- Line 203: Update each IconButton disabled prop in the table row-action
rendering, including the instances near the current expression and the
additional highlighted locations, to coerce the disable condition with !!. Match
the boolean coercion already used by getRowActions so every icon path passes a
boolean to IconButton.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e76a6e75-5930-4cee-8a49-64a1205c7f32
📒 Files selected for processing (38)
src/components/index.jssrc/components/mui/BulkEditTable/BulkEditTable.jssrc/components/mui/MuiBaseCustomTheme.jssrc/components/mui/SponsorOrderGrid/index.jssrc/components/mui/__tests__/fee-row.test.jssrc/components/mui/__tests__/mui-table-editable.test.jssrc/components/mui/__tests__/mui-table-sortable.test.jssrc/components/mui/__tests__/mui-table.test.jssrc/components/mui/__tests__/notes-row.test.jssrc/components/mui/__tests__/payment-row.test.jssrc/components/mui/__tests__/refund-row.test.jssrc/components/mui/__tests__/total-row.test.jssrc/components/mui/editable-table/mui-table-editable.jssrc/components/mui/editable-table/mui-table-editable.module.lesssrc/components/mui/sortable-table/mui-table-sortable.jssrc/components/mui/sortable-table/styles.module.lesssrc/components/mui/table/mui-table.jssrc/components/mui/tables/components/CustomTablePagination.jssrc/components/mui/tables/components/create-delete-handler.jssrc/components/mui/tables/components/row-actions-menu.jssrc/components/mui/tables/components/scroll-fade-overlay.jssrc/components/mui/tables/components/sortable-header-content.jssrc/components/mui/tables/components/styles.module.lesssrc/components/mui/tables/components/table-cell-content.jssrc/components/mui/tables/components/table-shell.jssrc/components/mui/tables/components/table-styles.jssrc/components/mui/tables/components/use-scroll-fade.jssrc/components/mui/tables/editable-table/index.jssrc/components/mui/tables/extra-rows/DiscountRow.jsxsrc/components/mui/tables/extra-rows/FeeRow.jsxsrc/components/mui/tables/extra-rows/NotesRow.jsxsrc/components/mui/tables/extra-rows/PaymentRow.jsxsrc/components/mui/tables/extra-rows/RefundRow.jsxsrc/components/mui/tables/extra-rows/TotalRow.jsxsrc/components/mui/tables/extra-rows/index.jssrc/components/mui/tables/mui-table/index.jssrc/components/mui/tables/sortable-table/index.jswebpack.common.js
💤 Files with no reviewable changes (5)
- src/components/mui/sortable-table/styles.module.less
- src/components/mui/editable-table/mui-table-editable.module.less
- src/components/mui/table/mui-table.js
- src/components/mui/sortable-table/mui-table-sortable.js
- src/components/mui/editable-table/mui-table-editable.js
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 12
🧹 Nitpick comments (1)
src/components/mui/tables/mui-table/index.js (1)
203-203: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCoerce
disabledto a boolean.
options.disableProp && row[options.disableProp]can evaluate tonull,undefined, a string, or a number.IconButtonexpects a boolean fordisabled.getRowActionsalready applies!!(...)for the same conditions, so the menu path and the icon path can disagree in typing. Apply!!at each icon button for consistency.♻️ Proposed change
- disabled={options.disableProp && row[options.disableProp]} + disabled={!!(options.disableProp && row[options.disableProp])}Also applies to: 234-238, 268-268, 293-293
🤖 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/mui/tables/mui-table/index.js` at line 203, Update each IconButton disabled prop in the table row-action rendering, including the instances near the current expression and the additional highlighted locations, to coerce the disable condition with !!. Match the boolean coercion already used by getRowActions so every icon path passes a boolean to IconButton.
🤖 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/mui/tables/components/create-delete-handler.js`:
- Around line 19-20: Update the dialog options in create-delete-handler to use
the showConfirmDialog API field iconType instead of type, preserving the
"warning" value so the delete confirmation displays its warning state.
- Around line 4-10: Update createDeleteHandler to accept a getId callback and
use it when invoking onDelete instead of always reading item.id. In
sortable-table, pass a callback that returns item[idKey], preserving the
configured row identifier for deletion while retaining existing behavior for the
default idKey.
In `@src/components/mui/tables/components/row-actions-menu.js`:
- Around line 28-34: Add a localized aria-label to the icon-only IconButton that
opens the row actions menu, using the component’s existing localization
mechanism and an existing translation key where available. Preserve the current
click behavior and data-testid.
In `@src/components/mui/tables/components/sortable-header-content.js`:
- Around line 15-17: Update the onClick handler in the sortable header content
to pass ascending order when col.columnKey differs from sortCol, and only negate
sortDir when the clicked column is already active. Preserve the existing column
key passed to onSort.
In `@src/components/mui/tables/components/styles.module.less`:
- Around line 7-12: Center the absolutely positioned border pseudo-element by
updating its positioning declarations: replace the top-zero alignment with top:
50% and apply a vertical translate matching half its 60% height. Remove the
ineffective align-self declaration while preserving the existing bottom, left,
border, and height styling.
In `@src/components/mui/tables/components/use-scroll-fade.js`:
- Around line 15-23: Update the React.useEffect around updateFades to observe
container and table dimension changes with a ResizeObserver, invoking
updateFades when either resizes. Observe the relevant container and table
elements, and disconnect the observer during cleanup alongside removing the
scroll listener.
In `@src/components/mui/tables/editable-table/index.js`:
- Around line 251-263: Update the editable-cell TableCell rendering around
isEditable, adding tabIndex only for editable cells and handling Enter or Space
keyboard activation by invoking handleCellClick with the row and column key.
Ignore keyboard events originating from the nested TextField, while preserving
non-editable cell behavior.
In `@src/components/mui/tables/mui-table/index.js`:
- Around line 115-119: Update the select action in the table component to use
the existing T.translate mechanism for the "View" label, including both the menu
entry and its tooltip. Add or reuse the appropriate translation key so both
visible labels follow the current locale while preserving the existing onSelect
behavior.
- Line 180: Update the className construction in the table cell rendering to
omit falsy or undefined values instead of interpolating them as strings.
Preserve both the conditional dotted-border class from col.dottedBorder and
col.className when present, while ensuring the rendered class list contains only
valid class names.
In `@src/components/mui/tables/sortable-table/index.js`:
- Around line 77-80: Use the configured idKey consistently for item identity in
the sortable table. Update the reorder logic around movedItemId and the
createDeleteHandler helper to derive IDs through a getId callback, then pass
(item) => item[idKey] when creating the delete handler so onReorder and onDelete
receive the configured item ID.
- Around line 182-217: Add localized aria-label values to the edit, delete, and
reorder IconButton elements in the table row actions, using the existing
localization mechanism and clear action-specific labels while preserving their
current click and drag behavior.
- Around line 66-75: Update the reorder logic around reordered and
updateOrderKey so assigning the new order does not mutate item objects supplied
through data. Create new item objects when applying updateOrderKey, while
preserving unchanged item references or values as appropriate, then pass the
resulting reordered collection to onReorder.
---
Nitpick comments:
In `@src/components/mui/tables/mui-table/index.js`:
- Line 203: Update each IconButton disabled prop in the table row-action
rendering, including the instances near the current expression and the
additional highlighted locations, to coerce the disable condition with !!. Match
the boolean coercion already used by getRowActions so every icon path passes a
boolean to IconButton.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e76a6e75-5930-4cee-8a49-64a1205c7f32
📒 Files selected for processing (38)
src/components/index.jssrc/components/mui/BulkEditTable/BulkEditTable.jssrc/components/mui/MuiBaseCustomTheme.jssrc/components/mui/SponsorOrderGrid/index.jssrc/components/mui/__tests__/fee-row.test.jssrc/components/mui/__tests__/mui-table-editable.test.jssrc/components/mui/__tests__/mui-table-sortable.test.jssrc/components/mui/__tests__/mui-table.test.jssrc/components/mui/__tests__/notes-row.test.jssrc/components/mui/__tests__/payment-row.test.jssrc/components/mui/__tests__/refund-row.test.jssrc/components/mui/__tests__/total-row.test.jssrc/components/mui/editable-table/mui-table-editable.jssrc/components/mui/editable-table/mui-table-editable.module.lesssrc/components/mui/sortable-table/mui-table-sortable.jssrc/components/mui/sortable-table/styles.module.lesssrc/components/mui/table/mui-table.jssrc/components/mui/tables/components/CustomTablePagination.jssrc/components/mui/tables/components/create-delete-handler.jssrc/components/mui/tables/components/row-actions-menu.jssrc/components/mui/tables/components/scroll-fade-overlay.jssrc/components/mui/tables/components/sortable-header-content.jssrc/components/mui/tables/components/styles.module.lesssrc/components/mui/tables/components/table-cell-content.jssrc/components/mui/tables/components/table-shell.jssrc/components/mui/tables/components/table-styles.jssrc/components/mui/tables/components/use-scroll-fade.jssrc/components/mui/tables/editable-table/index.jssrc/components/mui/tables/extra-rows/DiscountRow.jsxsrc/components/mui/tables/extra-rows/FeeRow.jsxsrc/components/mui/tables/extra-rows/NotesRow.jsxsrc/components/mui/tables/extra-rows/PaymentRow.jsxsrc/components/mui/tables/extra-rows/RefundRow.jsxsrc/components/mui/tables/extra-rows/TotalRow.jsxsrc/components/mui/tables/extra-rows/index.jssrc/components/mui/tables/mui-table/index.jssrc/components/mui/tables/sortable-table/index.jswebpack.common.js
💤 Files with no reviewable changes (5)
- src/components/mui/sortable-table/styles.module.less
- src/components/mui/editable-table/mui-table-editable.module.less
- src/components/mui/table/mui-table.js
- src/components/mui/sortable-table/mui-table-sortable.js
- src/components/mui/editable-table/mui-table-editable.js
🛑 Comments failed to post (1)
src/components/mui/tables/components/styles.module.less (1)
7-12: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Center the dotted border pseudo-element.
align-selfhas no effect on an absolutely positioned pseudo-element.The current border starts at the top. Use
top: 50%andtransformto center the 60% height border.Proposed fix
- top: 0; - bottom: 0; + top: 50%; left: 0; border-left: 1px dashed `#e0e0e0`; height: 60%; - align-self: center; + transform: translateY(-50%);📝 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.top: 50%; left: 0; border-left: 1px dashed `#e0e0e0`; height: 60%; transform: translateY(-50%);🤖 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/mui/tables/components/styles.module.less` around lines 7 - 12, Center the absolutely positioned border pseudo-element by updating its positioning declarations: replace the top-zero alignment with top: 50% and apply a vertical translate matching half its 60% height. Remove the ineffective align-self declaration while preserving the existing bottom, left, border, and height styling.
|
@santipalenque please review and fix code conflicts |
c7d45e5 to
f2798b7
Compare
https://app.clickup.com/t/9014802374/86bb7tvh1
Summary by CodeRabbit