Fix: responsive grids #1038
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,125 @@ | ||
| import React from "react"; | ||
| import PropTypes from "prop-types"; | ||
| import { Checkbox, FormControlLabel, FormGroup, Grid2 } from "@mui/material"; | ||
| import SearchInput from "openstack-uicore-foundation/lib/components/mui/search-input"; | ||
|
|
||
| const GridToolbar = ({ searchProps, checkboxProps, children, splitAt }) => { | ||
| const hasSearch = !!searchProps; | ||
| const hasCheckbox = !!checkboxProps; | ||
|
|
||
| let searchSize; | ||
| let checkboxSize; | ||
| let actionsSize; | ||
|
|
||
| if (hasSearch && hasCheckbox) { | ||
| searchSize = { xs: 12, sm: 6, md: 4 }; | ||
| checkboxSize = { xs: 12, sm: 6, md: 2 }; | ||
| actionsSize = { xs: 12, md: 6 }; | ||
| } else if (hasSearch) { | ||
| // has search but no checkbox | ||
| searchSize = { xs: 12, [splitAt]: 4 }; | ||
| actionsSize = { xs: 12, [splitAt]: 8 }; | ||
| } else if (hasCheckbox) { | ||
| // has checkbox but no search | ||
| checkboxSize = { xs: 12, [splitAt]: 4 }; | ||
| actionsSize = { xs: 12, [splitAt]: 8 }; | ||
| } else { | ||
| actionsSize = { xs: 12 }; | ||
| } | ||
|
|
||
| // must match the breakpoint where actionsSize itself leaves its xs:12 | ||
| // (own full-width row) value — that's the point flexWrap needs to switch | ||
| // to nowrap, so children don't get squeezed once actionsSize starts | ||
| // sharing a row with a sibling | ||
| const actionsWidthBreakpoint = | ||
| hasSearch && hasCheckbox ? "md" : hasSearch || hasCheckbox ? splitAt : "xs"; | ||
|
|
||
| // children go natural (auto) width starting at actionsWidthBreakpoint, | ||
| // never earlier than sm; between sm and that point (only a real window | ||
| // when actionsWidthBreakpoint is md) they fill the row evenly instead | ||
| const actionsAutoBreakpoint = | ||
| actionsWidthBreakpoint === "xs" ? "sm" : actionsWidthBreakpoint; | ||
| const hasFillTier = actionsAutoBreakpoint !== "sm"; | ||
|
|
||
| return ( | ||
| <Grid2 container spacing={2} sx={{ mb: 3 }}> | ||
| {hasSearch && ( | ||
| <Grid2 size={searchSize}> | ||
| <SearchInput {...searchProps} /> | ||
| </Grid2> | ||
| )} | ||
| {hasCheckbox && ( | ||
| <Grid2 size={checkboxSize}> | ||
| <FormGroup sx={{ flexShrink: 0 }}> | ||
| <FormControlLabel | ||
| control={ | ||
| <Checkbox | ||
| checked={checkboxProps.checked} | ||
| onChange={checkboxProps.onChange} | ||
| inputProps={{ | ||
| "aria-label": checkboxProps.ariaLabel ?? checkboxProps.label | ||
| }} | ||
|
Comment on lines
+59
to
+61
There was a problem hiding this comment. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
# Inspect GridToolbar checkbox consumers for non-string label expressions.
rg -n -P -U --glob '*.{js,jsx}' \
'checkboxProps=\{\{(?s:.{0,600}?label\s*:)' srcRepository: fntechgit/summit-admin Length of output: 4990 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- grid-toolbar.js ---'
sed -n '1,140p' src/components/mui/grid-toolbar.js
printf '%s\n' '--- checkboxProps and ariaLabel declarations/usages ---'
rg -n -C 4 'checkboxProps|ariaLabel|FormControlLabel|PropTypes\.node|PropTypes\.string' src/components/mui src | head -n 300
printf '%s\n' '--- translation helper definition ---'
rg -n -C 6 'translate\s*[:=]|function translate|const translate|translate\(' src | head -n 250Repository: fntechgit/summit-admin Length of output: 41240 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- GridToolbar call sites ---'
rg -n -C 12 '<GridToolbar\b|GridToolbar\s*\(' src --glob '*.{js,jsx}'
printf '%s\n' '--- translation packages and locale data ---'
rg -n 'i18n-react|T\.translate|translations|locale|locales' package.json package-lock.json yarn.lock src public config 2>/dev/null | head -n 250
printf '%s\n' '--- direct ariaLabel consumers ---'
rg -n -C 8 'ariaLabel' src --glob '*.{js,jsx}'Repository: fntechgit/summit-admin Length of output: 50379 🏁 Script executed: #!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
root = Path("src")
for path in sorted(root.rglob("*")):
if path.suffix not in {".js", ".jsx"}:
continue
text = path.read_text(errors="replace")
for match in re.finditer(r"<GridToolbar\b", text):
start = match.start()
# Extract the JSX element through its closing tag with a bounded scan.
end = text.find("</GridToolbar>", start)
if end < 0:
end = min(len(text), start + 2500)
else:
end += len("</GridToolbar>")
block = text[start:end]
has_checkbox = "checkboxProps" in block
labels = re.findall(r"\blabel\s*:\s*([^\n,}]+)", block)
aria = re.findall(r"\bariaLabel\s*:\s*([^\n,}]+)", block)
print(f"{path}: checkboxProps={has_checkbox}; labels={labels}; ariaLabels={aria}")
PYRepository: fntechgit/summit-admin Length of output: 4363 🌐 Web query:
💡 Result: The aria-label attribute in React DOM expects a string value [1][2]. Providing a ReactNode (such as JSX, an object, or an array) to aria-label is not valid and will not function as an accessible label for assistive technologies [3][4][2]. Key points regarding the aria-label attribute: 1. Expected Type: According to the WAI-ARIA specification and React implementation, aria-label is designed specifically to provide an accessible name via a string [3][1][4]. 2. React Behavior: React supports aria-* attributes directly in JSX using hyphen-case (kebab-case) [5][6][7]. While React allows dynamic values for these props, the underlying DOM attribute aria-label will coerce the provided value into a string [3][2]. Passing a ReactNode object (like a complex JSX element) will typically result in the attribute value being rendered as an inaccurate string representation (e.g., [object Object]), which fails to provide a meaningful label for screen readers [3][1]. 3. Best Practices: If you need to provide a complex label that includes non-text content, you should use aria-labelledby to reference a separate, visible element containing the full content, or ensure the aria-label is a simple, descriptive string [3][4][2]. In summary, aria-label must be a string [1][2]. It is not intended to hold ReactNode structures [3]. Citations:
🌐 Web query:
💡 Result: To ensure accessibility in MUI (Material UI), the following guidelines apply when using checkboxes and labels: 1. Recommended Approach: Use FormControlLabel The official MUI recommendation is to use the FormControlLabel component to provide a visible label for checkboxes, radio buttons, and switches [1][2][3]. This component wraps the control and the label, automatically associating them correctly for accessibility [4][5]. 2. Using aria-label when no visible label exists If a visible label cannot be used, you must provide an accessible name to the checkbox via the input element itself [1][6]. This is achieved by passing an aria-label, aria-labelledby, or title attribute to the input component using the slotProps.input property (or inputProps in older versions/specific components) [1][2][6][7]. Example of accessible label without visible text: <Checkbox slotProps={{ input: { 'aria-label': 'Descriptive label for screen readers' }, }} /> Key Takeaways: - All form controls, including checkboxes, should have an accessible label [1][6][3]. - FormControlLabel is the preferred method for displaying a visible label alongside a control [1][2]. - When a visible label is not possible, use slotProps.input (or inputProps) to apply an aria-label directly to the input element [1][6]. Citations:
Use only a string for
🤖 Prompt for AI Agents |
||
| /> | ||
| } | ||
| label={checkboxProps.label} | ||
| sx={{ whiteSpace: "nowrap" }} | ||
| /> | ||
| </FormGroup> | ||
| </Grid2> | ||
| )} | ||
| <Grid2 | ||
| size={actionsSize} | ||
| sx={{ | ||
| display: "flex", | ||
| justifyContent: "flex-end", | ||
| flexWrap: { xs: "wrap", [actionsWidthBreakpoint]: "nowrap" }, | ||
| gap: 2 | ||
| }} | ||
| > | ||
| {/* xs: stacked full width. sm through actionsAutoBreakpoint (only a | ||
| real window when that's md): fill the row evenly via flexGrow. | ||
| From actionsAutoBreakpoint on: natural/auto width. */} | ||
| {React.Children.map(children, (child) => | ||
| child | ||
| ? React.cloneElement(child, { | ||
| sx: { | ||
| width: { xs: "100%", [actionsAutoBreakpoint]: "auto" }, | ||
| ...(hasFillTier && { | ||
| flexGrow: { sm: 1, [actionsAutoBreakpoint]: 0 }, | ||
| flexBasis: { sm: 0, [actionsAutoBreakpoint]: "auto" } | ||
| }), | ||
| ...child.props.sx | ||
| } | ||
| }) | ||
| : child | ||
| )} | ||
| </Grid2> | ||
| </Grid2> | ||
| ); | ||
| }; | ||
|
|
||
| GridToolbar.propTypes = { | ||
| searchProps: PropTypes.shape({ | ||
| term: PropTypes.string, | ||
| onSearch: PropTypes.func.isRequired, | ||
| placeholder: PropTypes.string, | ||
| debounced: PropTypes.bool | ||
| }), | ||
| checkboxProps: PropTypes.shape({ | ||
| checked: PropTypes.bool, | ||
| onChange: PropTypes.func, | ||
| label: PropTypes.node, | ||
| ariaLabel: PropTypes.string | ||
| }), | ||
| // breakpoint where search/checkbox split from the actions row into their | ||
| // compact ratio — raise it (e.g. "lg") when actions holds a lot of children | ||
| splitAt: PropTypes.oneOf(["xs", "sm", "md", "lg", "xl"]) | ||
| }; | ||
|
|
||
| GridToolbar.defaultProps = { | ||
| searchProps: null, | ||
| checkboxProps: null, | ||
| splitAt: "sm" | ||
| }; | ||
|
|
||
| export default GridToolbar; | ||
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.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject
splitAt="xs"."xs"overwrites the intendedxs: 12values with 4 and 8. It also resolvesflexWrapto"nowrap"at xs. Multiple actions can overflow instead of stacking.Restrict
splitAttosmand larger breakpoints.Proposed fix
Also applies to: 34-41, 114-116
🤖 Prompt for AI Agents