fix: add field title for minimum and maximum fields - #884
Conversation
📝 WalkthroughWalkthroughMUI Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 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 docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms 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: 1
🧹 Nitpick comments (1)
src/components/mui/formik-inputs/additional-input/additional-input.js (1)
116-120: Same translation key used for labels and placeholders.Both
InputLabelcomponents use translation keys from theplaceholdersnamespace:
- Line 118:
"additional_inputs.placeholders.meta_field_minimum_quantity"- Line 134:
"additional_inputs.placeholders.meta_field_maximum_quantity"These same keys are reused for the
placeholderprops of theMuiFormikTextFieldcomponents (lines 123-124, 139-140). While this might be intentional to keep the text consistent, labels and placeholders typically serve different semantic purposes:
- Labels describe what the field is for (e.g., "Minimum Quantity")
- Placeholders provide example input or hints (e.g., "Enter minimum value")
💡 Consider using separate translation keys for labels
Create dedicated label translation keys:
- <InputLabel htmlFor={buildFieldName("minimum_quantity")}> - {T.translate( - "additional_inputs.placeholders.meta_field_minimum_quantity" - )} - </InputLabel> + <InputLabel htmlFor={buildFieldName("minimum_quantity")}> + {T.translate( + "additional_inputs.meta_field_minimum_quantity" + )} + </InputLabel>And similarly for maximum_quantity. This provides clearer separation and allows different text if needed in the future.
Also applies to: 132-136
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/mui/formik-inputs/additional-input/additional-input.js` around lines 116 - 120, The InputLabel components are using the "additional_inputs.placeholders.*" translation keys currently also used for field placeholders; update the labels to use distinct label keys (e.g., "additional_inputs.labels.meta_field_minimum_quantity" and "additional_inputs.labels.meta_field_maximum_quantity") and update the InputLabel calls in additional-input.js to reference those new keys instead of the placeholders keys; leave the MuiFormikTextField placeholder props referencing the existing placeholders keys so labels (InputLabel) and placeholders (MuiFormikTextField) are semantically separated and can be edited independently.
🤖 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/additional-input/additional-input.js`:
- Around line 116-120: The label-input association is broken because
InputLabel's htmlFor expects an explicit id on the underlying TextField; update
the MuiFormikTextField component to generate and pass a sanitized id derived
from the field name (e.g., implement a sanitizeId function that replaces
brackets and dots with hyphens) and set id={sanitizeId(name)} on the TextField,
and then update the parent that uses buildFieldName("minimum_quantity") so the
InputLabel uses the same sanitized id
(htmlFor={sanitizeId(buildFieldName("minimum_quantity"))}) to restore correct
accessibility linking.
---
Nitpick comments:
In `@src/components/mui/formik-inputs/additional-input/additional-input.js`:
- Around line 116-120: The InputLabel components are using the
"additional_inputs.placeholders.*" translation keys currently also used for
field placeholders; update the labels to use distinct label keys (e.g.,
"additional_inputs.labels.meta_field_minimum_quantity" and
"additional_inputs.labels.meta_field_maximum_quantity") and update the
InputLabel calls in additional-input.js to reference those new keys instead of
the placeholders keys; leave the MuiFormikTextField placeholder props
referencing the existing placeholders keys so labels (InputLabel) and
placeholders (MuiFormikTextField) are semantically separated and can be edited
independently.
🪄 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: 90546c47-487c-4c6e-a25b-414f4d6b2288
📒 Files selected for processing (1)
src/components/mui/formik-inputs/additional-input/additional-input.js
| <InputLabel htmlFor={buildFieldName("minimum_quantity")}> | ||
| {T.translate( | ||
| "additional_inputs.placeholders.meta_field_minimum_quantity" | ||
| )} | ||
| </InputLabel> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check how MuiFormikTextField handles the id attribute and verify TextField usage
# Check if MuiFormikTextField explicitly passes id prop to TextField
rg -n -A 10 'const MuiFormikTextField' src/components/mui/formik-inputs/mui-formik-textfield.js
# Search for any other instances where InputLabel with htmlFor is used with buildFieldName
rg -n -C 3 'htmlFor=\{buildFieldName' src/Repository: fntechgit/summit-admin
Length of output: 1799
🌐 Web query:
MUI TextField id generation from name prop with array notation bracket syntax
💡 Result:
No, MUI TextField does not generate the id from the name prop, including when using array notation bracket syntax like "items[0]". The id prop must be explicitly provided for proper accessibility linking between the label, input, and helper text. If no id is provided, recent versions (post-2021 proposals) may auto-generate a random id using React's useId hook, but it is not derived from the name prop. The name prop is forwarded to the underlying input element for form submission purposes, but id generation is independent. Official docs list id and name as separate props with no mention of automatic derivation. Source code inspection and GitHub issues/PRs confirm no such feature exists; proposals for auto-generation (not name-based) were discussed but marked duplicate or not merged as described. For array fields (e.g., name="items[0]"), developers typically manually set unique ids like id={items-${index}} when rendering dynamic lists, as seen in StackOverflow examples.
Citations:
- 1: https://mui.com/api/text-field/
- 2: https://mui.com/material-ui/api/text-field/
- 3: https://mui.com/material-ui/react-text-field/
- 4: [TextField] Generate id automatically mui/material-ui#29899
- 5: https://github.com/mui/material-ui/tree/v7.3.9/packages/mui-material/src/TextField
🌐 Web query:
HTML id attribute allowed characters restrictions brackets
💡 Result:
In HTML5 (current standard as of 2026), the id attribute has minimal restrictions: it must contain at least one character, must not contain any ASCII whitespace (space, tab, LF, FF, CR), and must be unique within the document. There are no other restrictions on characters, including brackets like [] or {}. Brackets (square brackets [], curly braces {}, parentheses ) are fully allowed in id values per the spec. MDN confirms the value "may contain any other Unicode character" except ASCII whitespace. However, special characters like brackets, ?, :, ., # require escaping in CSS selectors (e.g., #my[id]) or JavaScript querySelector (using CSS.escape), and may not be valid unquoted JS identifiers. For best compatibility with CSS/JS without escaping, use valid CSS identifiers: starting with letter/hyphen/underscore, followed by letters, digits, hyphens, underscores. HTML4 had stricter rules (start with letter, only specific chars), but HTML5 is permissive. Examples: - Valid: id="my-id[1]", id="test{}", id="123[]" - Invalid: id="", id="my id" (space), id with only whitespace.
Citations:
- 1: https://html.spec.whatwg.org/multipage/dom.html
- 2: https://html.spec.whatwg.org/dev/dom.html
- 3: https://developer.mozilla.org/en-US/docs/web/html/global_attributes/id
- 4: https://stackoverflow.com/questions/70579/html-valid-id-attribute-values
🌐 Web query:
Material-UI TextField accessibility htmlFor id linking formik array notation
💡 Result:
To achieve proper accessibility in Material-UI (MUI) TextField when used with Formik, especially in array notation (e.g., FieldArray with names like friends[0].name), follow these guidelines based on official documentation: ## Core Accessibility Requirement MUI TextField requires a unique id prop on the TextField component. This id is used internally to: - Link the InputLabel via htmlFor={id} (automatically handled by TextField). - Link the input element (id passed to ). - Link helper text via aria-describedby. Without an explicit id, labels won't associate properly with inputs for screen readers, violating WCAG 1.3.1 (Info and Relationships). Example DOM structure needed: Name Helper text ## With Formik Arrays Formik FieldArray uses dot/bracket notation for names: friends.${index} or friends[${index}]. The name prop handles form state binding, but does not automatically set id. ### Solution 1: Custom TextFieldProps Component (Recommended) Create a Formik-compatible wrapper that auto-generates id from name: jsx import React from 'react'; import { useField, FieldAttributes } from 'formik'; import { TextField } from '@mui/material'; interface FormikTextFieldProps { label: string; // other MUI props } const FormikMuiTextField: React.FC<FieldAttributes> = ({ label, ...props }) => { const [field, meta] = useField(props); const id = field-${field.name.replace(/[|]|./g, '-')}; // e.g., "field-friends-0-name" return ( <TextField {...field} id={id} label={label} error={meta.touched && Boolean(meta.error)} helperText={meta.touched && meta.error ? meta.error : undefined} {...props} /> ); }; Usage in FieldArray: jsx {({ push, remove }) => ( <> {values.friends.map((friend, index) => (
Citations:
- 1: https://mui.com/api/text-field
- 2: https://mui.com/material-ui/react-text-field.md
- 3: https://mui.com/material-ui/react-text-field/
- 4: https://formik.org/docs/api/fieldarray
- 5: https://formik.org/docs/api/field
- 6: https://mui.com/material-ui/react-text-field
- 7: [TextField] Generate id automatically mui/material-ui#29899
- 8: https://github.com/stackworx/formik-material-ui
Fix label-input association for accessibility: InputLabel htmlFor requires explicit id prop on TextField.
The MuiFormikTextField component must explicitly pass an id prop to the underlying MUI TextField. Currently, without an explicit id, MUI generates a random auto-id that won't match the htmlFor attribute on InputLabel, breaking the label-to-input association. This violates WCAG 1.3.1 (Info and Relationships) and prevents screen readers from properly associating labels with inputs.
Since the field name uses array notation like meta_fields[0].minimum_quantity, the id must be sanitized (replace brackets and dots with hyphens) to create a valid HTML id.
Solution: Modify MuiFormikTextField to generate and pass a sanitized id:
Example fix
// In MuiFormikTextField component
const sanitizeId = (name) => name.replace(/[\[\]\.]/g, '-');
<TextField
id={sanitizeId(name)}
name={name}
// ... other props
/>Then in the parent component, ensure InputLabel references the same sanitized id:
<InputLabel htmlFor={sanitizeId(buildFieldName("minimum_quantity"))}>
{T.translate("additional_inputs.placeholders.meta_field_minimum_quantity")}
</InputLabel>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/mui/formik-inputs/additional-input/additional-input.js` around
lines 116 - 120, The label-input association is broken because InputLabel's
htmlFor expects an explicit id on the underlying TextField; update the
MuiFormikTextField component to generate and pass a sanitized id derived from
the field name (e.g., implement a sanitizeId function that replaces brackets and
dots with hyphens) and set id={sanitizeId(name)} on the TextField, and then
update the parent that uses buildFieldName("minimum_quantity") so the InputLabel
uses the same sanitized id
(htmlFor={sanitizeId(buildFieldName("minimum_quantity"))}) to restore correct
accessibility linking.
28b8c9d to
801147d
Compare
|
this was replaced by OpenStackweb/openstack-uicore-foundation#230 |
ref: https://app.clickup.com/t/86b8ejhum
Summary by CodeRabbit