Open
Conversation
📝 WalkthroughWalkthroughThe PR adds customization capabilities for delete confirmation dialogs by introducing dynamic styling for confirm buttons and exposing new props to configure confirmation text and button color across components using MuiTable. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
priscila-moneo
force-pushed
the
fix/change-delete-button-to-confirm
branch
from
0a2e9c2 to
be33b82
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/mui/table/mui-table.js (1)
88-90: Consider nullish coalescing for fallback semantics.Using
??avoids treating intentionally empty strings as missing values.Proposed tweak
- confirmButtonColor: confirmButtonColor || "error", + confirmButtonColor: confirmButtonColor ?? "error", confirmButtonText: - deleteDialogConfirmText || T.translate("general.yes_delete") + deleteDialogConfirmText ?? T.translate("general.yes_delete")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/mui/table/mui-table.js` around lines 88 - 90, The fallback logic for confirmButtonColor and confirmButtonText uses || which treats empty strings as missing; update the assignments in the component (confirmButtonColor and confirmButtonText derived from deleteDialogConfirmText and T.translate) to use the nullish coalescing operator (??) so only null or undefined fall back (e.g., confirmButtonColor = confirmButtonColor ?? "error" and confirmButtonText = deleteDialogConfirmText ?? T.translate("general.yes_delete")).
🤖 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/mui/confirm-dialog.js`:
- Around line 60-78: The Button is receiving confirmButtonColor even when it may
be a hex string which violates the MUI v6 Button.color API; update the
ConfirmDialog Button rendering so that the color prop is only set when
confirmButtonColor is a palette key (non-hex), e.g. set color={typeof
confirmButtonColor==="string" && !confirmButtonColor.startsWith("#") ?
confirmButtonColor : undefined}, and keep the existing sx augmentation block to
apply hex colors (augmentColor logic) only when confirmButtonColor
startsWith("#"); ensure you reference the confirmButtonColor prop and the
Button's color and sx props in this change.
---
Nitpick comments:
In `@src/components/mui/table/mui-table.js`:
- Around line 88-90: The fallback logic for confirmButtonColor and
confirmButtonText uses || which treats empty strings as missing; update the
assignments in the component (confirmButtonColor and confirmButtonText derived
from deleteDialogConfirmText and T.translate) to use the nullish coalescing
operator (??) so only null or undefined fall back (e.g., confirmButtonColor =
confirmButtonColor ?? "error" and confirmButtonText = deleteDialogConfirmText ??
T.translate("general.yes_delete")).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d757365a-1bf2-4245-8a19-695bebc31b5d
📒 Files selected for processing (3)
src/components/mui/confirm-dialog.jssrc/components/mui/table/mui-table.jssrc/pages/sponsors/sponsor-users-list-page/components/request-table.js
src/components/mui/confirm-dialog.js
Outdated
Comment on lines
+60
to
+78
| color={confirmButtonColor} | ||
| sx={ | ||
| typeof confirmButtonColor === "string" && | ||
| confirmButtonColor.startsWith("#") | ||
| ? (theme) => { | ||
| const augmentedColor = theme.palette.augmentColor({ | ||
| color: { main: confirmButtonColor } | ||
| }); | ||
|
|
||
| return { | ||
| backgroundColor: augmentedColor.main, | ||
| color: augmentedColor.contrastText, | ||
| "&:hover": { | ||
| backgroundColor: augmentedColor.dark | ||
| } | ||
| }; | ||
| } | ||
| : undefined | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
MUI v6 Material-UI Button color prop hex string support documentation
💡 Result:
MUI v6 <Button color={...}> does not document support for passing a raw hex value (for example color="#ff0000"). The Button API documents color as a set of theme palette keys ('primary' | 'secondary' | 'success' | ...')—with string intended for theme-extended custom color names, not arbitrary CSS colors. [1]
If you want a hex-based color, the documented approaches are:
- Put the hex in the theme palette (tokens like
main,light,dark,contrastTextsupporthex/rgb/hsl), then use that palette key viacolor="yourColorName"(with TS module augmentation for thecolorprop). [2] - Style the button directly (for example via
sx={{ bgcolor: '#ff0000' }}/sx={{ color: '#fff' }}), rather than using thecolorprop. (This is implied bycolorbeing palette-key-based in the API.) [1]
References
- MUI v6 Button API (
colorprop types and description) — https://mui.com/material-ui/api/button/ - MUI v6 Palette customization (custom colors, supported formats, using in components, TS overrides) — https://mui.com/material-ui/customization/palette/
Do not pass hex values through Button.color—it violates the MUI v6 API contract.
MUI v6 Button's color prop officially supports palette keys only ('primary', 'secondary', etc.), not arbitrary hex strings. Line 60 passes confirmButtonColor to color, which may be a hex string, contradicting the documented API. The correct approach is to apply hex styling exclusively via sx when detected, leaving color undefined for non-palette values.
Proposed refactor
+const isHexConfirmColor =
+ typeof confirmButtonColor === "string" &&
+ confirmButtonColor.startsWith("#");
...
<Button
fullWidth
onClick={onConfirm}
variant="contained"
- color={confirmButtonColor}
+ color={isHexConfirmColor ? undefined : confirmButtonColor}
sx={
- typeof confirmButtonColor === "string" &&
- confirmButtonColor.startsWith("#")
+ isHexConfirmColor
? (theme) => {
const augmentedColor = theme.palette.augmentColor({
color: { main: confirmButtonColor }
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/mui/confirm-dialog.js` around lines 60 - 78, The Button is
receiving confirmButtonColor even when it may be a hex string which violates the
MUI v6 Button.color API; update the ConfirmDialog Button rendering so that the
color prop is only set when confirmButtonColor is a palette key (non-hex), e.g.
set color={typeof confirmButtonColor==="string" &&
!confirmButtonColor.startsWith("#") ? confirmButtonColor : undefined}, and keep
the existing sx augmentation block to apply hex colors (augmentColor logic) only
when confirmButtonColor startsWith("#"); ensure you reference the
confirmButtonColor prop and the Button's color and sx props in this change.
be33b82 to
3b81d8b
Compare
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
2 participants
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.
ref: https://app.clickup.com/t/86b81f7x4
mui-table.jsto support per-use customization of delete confirmation button text (deleteDialogConfirmText) and color (confirmButtonColor), while keeping defaults.request-table.js: button text is "Confirm" and color is primary.Summary by CodeRabbit