feat: add new mui formik color picker component#231
Open
Conversation
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a client-side filtering mode to ChangesAsync Select: localFilter mode
Color Input component + exports / build entry
Sequence DiagramsequenceDiagram
participant User as User
participant Component as MuiFormikAsyncAutocomplete
participant API as Remote API
Note over Component: Mode = localFilter ? local : remote
User->>Component: Type input
alt localFilter == true
Component->>Component: filterOptions(options, input) (case-insensitive substring)
Component-->>User: show filtered options
else remote mode
Component->>Component: debounce input -> set searchTerm
Component->>API: fetchOptions(searchTerm)
API-->>Component: return options
Component-->>User: show fetched options
end
User->>Component: Select option
Component-->>User: emit selection / update Formik
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.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: 4
🤖 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/formik-inputs/mui-formik-async-select.js`:
- Around line 34-35: The component defines and accepts isMulti but still uses
the older multiple flag when computing Autocomplete value and onChange, causing
Formik to store the wrong shape; update all selection/value logic to
consistently use isMulti (not multiple) — pass isMulti to the MUI Autocomplete
prop, branch on isMulti when deriving value from Formik field and when calling
setFieldValue (map arrays for multi-select, single item otherwise), and
remove/rename any references to multiple so Formik and Autocomplete agree on the
value shape (check functions/variables around isMulti, Autocomplete usage, and
the onChange/value handlers).
- Around line 107-110: The local filtering callback currently calls
opt.label.toLowerCase() which will throw if opt.label is
null/undefined/non-string; update the filter used in options.filter((opt) =>
...) to coerce or guard the label and inputValue (e.g., use String(opt.label ??
'') or (opt.label || '').toString()) and similarly normalize inputValue before
calling toLowerCase(), then perform the includes check; ensure you update the
inline filter expression where options.filter is defined to be null-safe for
labels.
In `@src/components/mui/formik-inputs/mui-formik-color-input.js`:
- Line 8: The localValue state initialized with useState(field.value ||
"#000000") can become stale on form resets; add an effect that listens for
changes to field.value (the Formik value) and calls setLocalValue(field.value ??
"#000000") to keep the color swatch in sync; locate the localValue and
setLocalValue declarations in mui-formik-color-input.js and add a useEffect
referencing field.value to update localValue when Formik reinitializes or
resets.
- Around line 20-34: The TextField currently spreads {...rest} last which allows
callers to override controlled props (type, value, onChange, onBlur) and bypass
Formik; update the component that renders TextField so you either spread
{...rest} first (before value={localValue}, onChange={handleChange},
onBlur={handleBlur}, type="color") or, preferably, strip conflicting keys from
rest (delete or omit 'type','value','onChange','onBlur') before spreading into
TextField; ensure the retained props are localValue, handleChange, handleBlur,
and the type remains "color" so Formik binding (meta.touched/meta.error) cannot
be overridden.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c5aedc0a-4634-4a6b-9aa5-2640867c3e16
📒 Files selected for processing (2)
src/components/mui/formik-inputs/mui-formik-async-select.jssrc/components/mui/formik-inputs/mui-formik-color-input.js
Comment on lines
+34
to
+35
| isMulti = false, | ||
| defaultOptions |
There was a problem hiding this comment.
Unify multiple control to avoid broken multi-select value handling.
Line 34 introduces isMulti, but selection/value logic still branches on multiple. This can put Autocomplete in multi mode while Formik still stores a single value.
Suggested fix
const MuiFormikAsyncAutocomplete = ({
name,
queryFunction,
multiple = false,
@@
- queryParams = [],
- isMulti = false,
+ queryParams = [],
defaultOptions
}) => {
@@
<Autocomplete
options={options}
value={value}
onChange={handleChange}
loading={loading}
- multiple={isMulti}
+ multiple={multiple}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/mui/formik-inputs/mui-formik-async-select.js` around lines 34
- 35, The component defines and accepts isMulti but still uses the older
multiple flag when computing Autocomplete value and onChange, causing Formik to
store the wrong shape; update all selection/value logic to consistently use
isMulti (not multiple) — pass isMulti to the MUI Autocomplete prop, branch on
isMulti when deriving value from Formik field and when calling setFieldValue
(map arrays for multi-select, single item otherwise), and remove/rename any
references to multiple so Formik and Autocomplete agree on the value shape
(check functions/variables around isMulti, Autocomplete usage, and the
onChange/value handlers).
Comment on lines
+107
to
+110
| ? (options, { inputValue }) => | ||
| options.filter((opt) => | ||
| opt.label.toLowerCase().includes(inputValue.toLowerCase()) | ||
| ) |
There was a problem hiding this comment.
Make local filtering null-safe for option labels.
This path calls toLowerCase() directly on opt.label; a non-string/empty label will throw at runtime.
Suggested fix
? (options, { inputValue }) =>
options.filter((opt) =>
- opt.label.toLowerCase().includes(inputValue.toLowerCase())
+ (opt.label ?? "")
+ .toString()
+ .toLowerCase()
+ .includes((inputValue ?? "").toLowerCase())
)📝 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.
Suggested change
| ? (options, { inputValue }) => | |
| options.filter((opt) => | |
| opt.label.toLowerCase().includes(inputValue.toLowerCase()) | |
| ) | |
| ? (options, { inputValue }) => | |
| options.filter((opt) => | |
| (opt.label ?? "") | |
| .toString() | |
| .toLowerCase() | |
| .includes((inputValue ?? "").toLowerCase()) | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/mui/formik-inputs/mui-formik-async-select.js` around lines 107
- 110, The local filtering callback currently calls opt.label.toLowerCase()
which will throw if opt.label is null/undefined/non-string; update the filter
used in options.filter((opt) => ...) to coerce or guard the label and inputValue
(e.g., use String(opt.label ?? '') or (opt.label || '').toString()) and
similarly normalize inputValue before calling toLowerCase(), then perform the
includes check; ensure you update the inline filter expression where
options.filter is defined to be null-safe for labels.
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
smarcet
reviewed
May 4, 2026
smarcet
reviewed
May 4, 2026
smarcet
reviewed
May 4, 2026
smarcet
reviewed
May 4, 2026
smarcet
reviewed
May 4, 2026
smarcet
reviewed
May 4, 2026
…ments Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/components/mui/formik-inputs/mui-formik-async-select.js (2)
115-118:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
opt.label.toLowerCase()still throws on null/undefined labels.The
filterOptionscallback can crash if any option has a missing label. This was flagged in a previous review and remains unresolved.🤖 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/formik-inputs/mui-formik-async-select.js` around lines 115 - 118, The filterOptions callback in mui-formik-async-select.js can throw when an option has a null/undefined label; update the filter function used by filterOptions (the arrow function in that file) to defensively coerce the label to a string before calling toLowerCase (e.g., use (opt && (opt.label ?? '')) or String(opt?.label ?? '') then toLowerCase) and also guard against opt being falsy, so options with missing labels no longer crash the filtering logic.
42-43:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
isMulti/multipleduality persists —AutocompleteandhandleChangestill disagree on multi-select shape.Line 107 passes
multiple={isMulti}toAutocomplete, whilehandleChange(line 84) still branches onmultiple. WhenisMulti=truebutmultiple=false(the default), Formik receives a single value while MUI is in multi-select mode, producing a broken experience.🤖 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/formik-inputs/mui-formik-async-select.js` around lines 42 - 43, The component mixes two names for the same concept: use the single prop name isMulti everywhere to avoid shape mismatches. Update the handleChange function to branch on isMulti (not multiple), ensure the Autocomplete is passed multiple={isMulti}, and remove/normalize any use of a separate multiple prop in the component props/destructuring so Formik always receives the value shape that matches Autocomplete's multiple={isMulti} behavior.
🧹 Nitpick comments (1)
src/components/mui/formik-inputs/mui-formik-async-select.js (1)
158-164: ⚡ Quick winPropTypes block is incomplete — 6 of 11 props are undocumented.
The following props accepted by the component are absent from the
propTypesdefinition:multiple,placeholder,plainValue,hiddenOptions,formatSelectedValue, andisMulti. MissingisMultiis especially risky given the existingisMulti/multipleconfusion, as dev-mode warnings won't catch type mismatches on either prop.♻️ Proposed completion
MuiFormikAsyncAutocomplete.propTypes = { name: PropTypes.string.isRequired, queryFunction: PropTypes.func.isRequired, formatOption: PropTypes.func, + formatSelectedValue: PropTypes.func, queryParams: PropTypes.array, + hiddenOptions: PropTypes.array, + multiple: PropTypes.bool, + isMulti: PropTypes.bool, + plainValue: PropTypes.bool, localFilter: PropTypes.bool, + placeholder: PropTypes.string, };🤖 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/formik-inputs/mui-formik-async-select.js` around lines 158 - 164, The propTypes for MuiFormikAsyncAutocomplete are missing six props (multiple, placeholder, plainValue, hiddenOptions, formatSelectedValue, isMulti); update the MuiFormikAsyncAutocomplete.propTypes object to include these properties with appropriate types (multiple: PropTypes.bool, isMulti: PropTypes.bool, placeholder: PropTypes.string, plainValue: PropTypes.bool, hiddenOptions: PropTypes.array, formatSelectedValue: PropTypes.func) so React dev-mode warnings catch misuse, and ensure the component’s defaultProps (if any) and internal logic treat isMulti and multiple consistently.
🤖 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.
Duplicate comments:
In `@src/components/mui/formik-inputs/mui-formik-async-select.js`:
- Around line 115-118: The filterOptions callback in mui-formik-async-select.js
can throw when an option has a null/undefined label; update the filter function
used by filterOptions (the arrow function in that file) to defensively coerce
the label to a string before calling toLowerCase (e.g., use (opt && (opt.label
?? '')) or String(opt?.label ?? '') then toLowerCase) and also guard against opt
being falsy, so options with missing labels no longer crash the filtering logic.
- Around line 42-43: The component mixes two names for the same concept: use the
single prop name isMulti everywhere to avoid shape mismatches. Update the
handleChange function to branch on isMulti (not multiple), ensure the
Autocomplete is passed multiple={isMulti}, and remove/normalize any use of a
separate multiple prop in the component props/destructuring so Formik always
receives the value shape that matches Autocomplete's multiple={isMulti}
behavior.
---
Nitpick comments:
In `@src/components/mui/formik-inputs/mui-formik-async-select.js`:
- Around line 158-164: The propTypes for MuiFormikAsyncAutocomplete are missing
six props (multiple, placeholder, plainValue, hiddenOptions,
formatSelectedValue, isMulti); update the MuiFormikAsyncAutocomplete.propTypes
object to include these properties with appropriate types (multiple:
PropTypes.bool, isMulti: PropTypes.bool, placeholder: PropTypes.string,
plainValue: PropTypes.bool, hiddenOptions: PropTypes.array, formatSelectedValue:
PropTypes.func) so React dev-mode warnings catch misuse, and ensure the
component’s defaultProps (if any) and internal logic treat isMulti and multiple
consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 043e6bbb-fe41-4edc-962a-955e38345598
📒 Files selected for processing (4)
src/components/index.jssrc/components/mui/formik-inputs/mui-formik-async-select.jssrc/components/mui/formik-inputs/mui-formik-color-input.jswebpack.common.js
✅ Files skipped from review due to trivial changes (1)
- src/components/index.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/mui/formik-inputs/mui-formik-color-input.js
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
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/86b9n7p5w
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit