-
Notifications
You must be signed in to change notification settings - Fork 4
Feature/mui components improvements #224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,14 +12,13 @@ | |
| * */ | ||
|
|
||
| import React from "react"; | ||
| import { MenuItem } from "@mui/material"; | ||
| import MuiFormikCheckbox from "../../formik-inputs/mui-formik-checkbox"; | ||
| import MuiFormikDropdownCheckbox from "../../formik-inputs/mui-formik-dropdown-checkbox"; | ||
| import MuiFormikDropdownRadio from "../../formik-inputs/mui-formik-dropdown-radio"; | ||
| import MuiFormikDatepicker from "../../formik-inputs/mui-formik-datepicker"; | ||
| import MuiFormikTimepicker from "../../formik-inputs/mui-formik-timepicker"; | ||
| import MuiFormikTextField from "../../formik-inputs/mui-formik-textfield"; | ||
| import MuiFormikSelect from "../../formik-inputs/mui-formik-select"; | ||
| import MuiFormikSelectV2 from "../../formik-inputs/mui-formik-select-v2"; | ||
|
|
||
| const ItemTableField = ({ | ||
| rowId, | ||
|
|
@@ -33,7 +32,7 @@ const ItemTableField = ({ | |
|
|
||
| switch (field.type) { | ||
| case "CheckBox": | ||
| return <MuiFormikCheckbox {...commonProps} />; | ||
| return <MuiFormikCheckbox {...commonProps} size="small" />; | ||
| case "CheckBoxList": | ||
| return ( | ||
| <MuiFormikDropdownCheckbox | ||
|
|
@@ -73,13 +72,11 @@ const ItemTableField = ({ | |
| ); | ||
| case "ComboBox": | ||
| return ( | ||
| <MuiFormikSelect {...commonProps} size="small"> | ||
| {field.values.map((v) => ( | ||
| <MenuItem key={`ddopt-${v.id}`} value={v.id}> | ||
| {v.value} | ||
| </MenuItem> | ||
| ))} | ||
| </MuiFormikSelect> | ||
| <MuiFormikSelectV2 | ||
| {...commonProps} | ||
| size="small" | ||
| options={field.values.map((v) => ({ value: v.id, label: v.value }))} | ||
| /> | ||
|
Comment on lines
+75
to
+79
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: Yes, in MUI v6, when using Select with displayEmpty={true} and an InputLabel, the label needs to be forced to shrink (via shrink={true} on InputLabel) to avoid overlapping with the empty placeholder value. The official Select API documentation explicitly warns: "When using this prop, make sure the label doesn't overlap with the empty displayed value. The label should either be hidden or forced to a shrunk state." For outlined variant, you may also need notched={true} on the Select to properly notch the border. Citations:
🏁 Script executed: # First, find and read the ItemTableField.js file around lines 75-79
fd -t f "ItemTableField.js" | head -5Repository: OpenStackweb/openstack-uicore-foundation Length of output: 145 🏁 Script executed: # Also search for the mui-formik-select-v2.js file
fd -t f "mui-formik-select-v2.js" | head -5Repository: OpenStackweb/openstack-uicore-foundation Length of output: 140 🏁 Script executed: # Check for MUI version in package.json
fd -t f "package.json" -x grep -l "\"@mui" {} \;Repository: OpenStackweb/openstack-uicore-foundation Length of output: 98 🏁 Script executed: # Read ItemTableField.js around lines 75-79
cat -n src/components/mui/FormItemTable/components/ItemTableField.js | sed -n '70,85p'Repository: OpenStackweb/openstack-uicore-foundation Length of output: 598 🏁 Script executed: # Read the entire mui-formik-select-v2.js file
wc -l src/components/mui/formik-inputs/mui-formik-select-v2.jsRepository: OpenStackweb/openstack-uicore-foundation Length of output: 143 🏁 Script executed: # Check MUI version in package.json
grep -A 2 "@mui/material" package.json | head -10Repository: OpenStackweb/openstack-uicore-foundation Length of output: 300 🏁 Script executed: # Read the full mui-formik-select-v2.js file
cat -n src/components/mui/formik-inputs/mui-formik-select-v2.jsRepository: OpenStackweb/openstack-uicore-foundation Length of output: 2129 🏁 Script executed: # Search for other occurrences of shrink in dropdown/select related files
rg -i "shrink" src/components/mui/formik-inputs/ -A 2 -B 2Repository: OpenStackweb/openstack-uicore-foundation Length of output: 4092 🏁 Script executed: # Search for displayEmpty usage
rg "displayEmpty" src/components/mui/Repository: OpenStackweb/openstack-uicore-foundation Length of output: 696 Add The component uses Fix in
|
||
| ); | ||
| case "Text": | ||
| return <MuiFormikTextField {...commonProps} fullWidth size="small" />; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| import React from "react"; | ||
| import PropTypes from "prop-types"; | ||
| import T from "i18n-react/dist/i18n-react"; | ||
| import { | ||
| Select, | ||
| FormHelperText, | ||
| FormControl, | ||
| MenuItem, | ||
| InputLabel | ||
| } from "@mui/material"; | ||
| import { useField } from "formik"; | ||
|
|
||
| const MuiFormikSelectV2 = ({ name, label, placeholder, options, ...rest }) => { | ||
| const [field, meta] = useField(name); | ||
| const finalPlaceholder = | ||
| placeholder || T.translate("general.select_an_option"); | ||
|
|
||
| return ( | ||
| <FormControl fullWidth error={meta.touched && Boolean(meta.error)}> | ||
| {label && <InputLabel id={`${name}-label`}>{label}</InputLabel>} | ||
| <Select | ||
| name={name} | ||
| label={label} | ||
| labelId={`${name}-label`} | ||
| displayEmpty | ||
| // eslint-disable-next-line react/jsx-props-no-spreading | ||
| {...field} | ||
| // eslint-disable-next-line react/jsx-props-no-spreading | ||
| {...rest} | ||
| renderValue={(selected) => { | ||
| if (selected == null || selected === "") { | ||
| return <em>{finalPlaceholder}</em>; | ||
| } | ||
| const selectedOption = options.find( | ||
| ({ value }) => value === selected | ||
| ); | ||
| return selectedOption ? selectedOption.label : ""; | ||
| }} | ||
| > | ||
| {options.map((op) => ( | ||
| <MenuItem key={`selectop-${op.value}`} value={op.value}> | ||
| {op.label} | ||
| </MenuItem> | ||
| ))} | ||
| </Select> | ||
| {meta.touched && meta.error && ( | ||
| <FormHelperText>{meta.error}</FormHelperText> | ||
| )} | ||
| </FormControl> | ||
| ); | ||
| }; | ||
|
|
||
| MuiFormikSelectV2.propTypes = { | ||
| name: PropTypes.string.isRequired, | ||
| options: PropTypes.array.isRequired | ||
| }; | ||
|
|
||
| export default MuiFormikSelectV2; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate
data-testid="select"collides with the existing mock.Both
mui-formik-select(Line 76) andmui-formik-select-v2(Line 87) now renderdata-testid="select". Any test that renders both (or queries withgetAllByTestId) becomes ambiguous, and the ComboBox test at Line 138 no longer clearly targets v2. Use a distinct id such as"select-v2"and update the corresponding assertion/title.Proposed fix
jest.mock("../../formik-inputs/mui-formik-select-v2", () => { const React = require("react"); return { __esModule: true, - default: ({ name }) => <div data-testid="select" data-name={name} /> + default: ({ name }) => <div data-testid="select-v2" data-name={name} /> }; });And update the ComboBox test:
📝 Committable suggestion
🤖 Prompt for AI Agents