Skip to content

feat: improve mui components #215

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.

Sign up for GitHub

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

Merged
smarcet merged 1 commit into from
Apr 9, 2026
+28 −4
Merged

feat: improve mui components #215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/components/mui/SnackbarNotification/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import CheckCircleIcon from "@mui/icons-material/CheckCircle";
import ErrorIcon from "@mui/icons-material/Error";
import SnackbarNotificationContext from "./Context";
import { NOTIFICATION_TIMEOUT } from "../../../utils/constants";
import { empty } from "../../../utils/methods";
import { clearSnackbarMessage } from "../../../utils/actions";

/*
Expand Down Expand Up @@ -50,14 +51,16 @@ const SnackbarNotification = ({
};

useEffect(() => {
if (msgData.html) {
if (!empty(msgData.html)) {
setOpen(true);
} else {
setOpen(false);
}
}, [msgData]);

// when snackbarMessage changes in base-reducer, we trigger the snackbar
useEffect(() => {
if (snackbarMessage?.html) {
if (!empty(snackbarMessage?.html)) {
setMsgData(snackbarMessage);
}
Comment on lines +63 to 65
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Redux clear events are no longer synced to local msgData.

Line 63 only updates local state when snackbarMessage?.html is non-empty. When CLEAR_SNACKBAR_MESSAGE resets html to "", local msgData is not cleared, so stale content can persist until another local action closes it.

🛠️ Proposed fix 
  useEffect(() => {
    if (!empty(snackbarMessage?.html)) {
      setMsgData(snackbarMessage);
+    } else {
+      setMsgData({});
     }
  }, [snackbarMessage]);
📝 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
if (!empty(snackbarMessage?.html)) {
setMsgData(snackbarMessage);
}
if (!empty(snackbarMessage?.html)) {
setMsgData(snackbarMessage);
} else {
setMsgData({});
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/components/mui/SnackbarNotification/index.js` around lines 63 - 65, The
local state msgData isn't cleared when snackbarMessage.html is reset to "", so
update the handler around snackbarMessage (the code using snackbarMessage?.html
and setMsgData) to synchronize clears: always call setMsgData(snackbarMessage)
(or call setMsgData(null) / {} in the else branch) when snackbarMessage becomes
empty or falsy so CLEAR_SNACKBAR_MESSAGE from Redux clears the local msgData;
adjust the effect or handler that reads snackbarMessage to
setMsgData(snackbarMessage) unconditionally or add an explicit else path to
clear msgData.

}, [snackbarMessage]);
Expand Down
13 changes: 11 additions & 2 deletions src/components/mui/dropdown-checkbox.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,16 @@ const DropdownCheckbox = ({
allLabel,
value = [],
options,
onChange
onChange,
...rest
}) => {
const handleChange = (ev) => {
const selected = ev.target.value;
const rawValue = ev.target.value;
const selected = Array.isArray(rawValue)
? rawValue
: typeof rawValue === "string"
? rawValue.split(",")
: [];
Comment on lines +36 to +41
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In MUI v6 Select with multiple, when can event.target.value be a string, and what does MUI recommend for handling value types when option values are numbers?

💡 Result:

  • With Select multiple, event.target.value is only a string when the change is coming from browser autofill (otherwise it’s the normal “generic event” MUI fires, and the value you read is the array). MUI explicitly calls out that the onChange event is not a change event unless caused by autofill. [1] The type definition reflects this by allowing SelectChangeEvent to also be a React.ChangeEvent<HTMLInputElement> (whose target.value is a string). [2]

  • If your option values are numbers, MUI’s guidance implies you should handle the fact that selection matching is done via string representation (so types must stay consistent), and therefore you should convert the autofill string back into the numeric type you store (e.g., split and map(Number) for multiple). [1]

Sources: [1] Select API (onChange autofill note; value/string-representation matching), [2] SelectChangeEvent type (union including React.ChangeEvent<HTMLInputElement>)

🏁 Script executed:

cat -n src/components/mui/dropdown-checkbox.js | head -60

Repository: OpenStackweb/openstack-uicore-foundation

Length of output: 2253

🏁 Script executed:

cat -n src/components/mui/dropdown-checkbox.js | tail -n +60

Repository: OpenStackweb/openstack-uicore-foundation

Length of output: 1535

🏁 Script executed:

rg "DropdownCheckbox" --type js --type tsx -B 2 -A 5 | head -80

Repository: OpenStackweb/openstack-uicore-foundation

Length of output: 111

🏁 Script executed:

rg "DropdownCheckbox" --type js -B 2 -A 5 | head -100

Repository: OpenStackweb/openstack-uicore-foundation

Length of output: 8121

🏁 Script executed:

cat -n src/components/mui/__tests__/dropdown-checkbox.test.js

Repository: OpenStackweb/openstack-uicore-foundation

Length of output: 2252

Normalize option ID types when converting ev.target.value in autofill scenarios.

When MUI Select fires onChange from browser autofill, ev.target.value is a string that must be split into an array. However, if option IDs are numeric, this creates a type mismatch: the selected array contains strings while the downstream checks at line 77 (selected.includes(id)) and line 89 (value.includes(id)) expect numeric types. This breaks both the rendered display and checked state after autofill.

Proposed fix 
   const handleChange = (ev) => {
     const rawValue = ev.target.value;
-    const selected = Array.isArray(rawValue)
-      ? rawValue
-      : typeof rawValue === "string"
-      ? rawValue.split(",")
-      : [];
+    const selected = (Array.isArray(rawValue)
+      ? rawValue
+      : typeof rawValue === "string"
+      ? rawValue.split(",")
+      : []
+    ).map((v) => {
+      if (v === "all") return "all";
+      const match = options.find(({ id }) => String(id) === String(v));
+      return match ? match.id : v;
+    });
📝 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
const rawValue = ev.target.value;
const selected = Array.isArray(rawValue)
? rawValue
: typeof rawValue === "string"
? rawValue.split(",")
: [];
const rawValue = ev.target.value;
const selected = (Array.isArray(rawValue)
? rawValue
: typeof rawValue === "string"
? rawValue.split(",")
: []
).map((v) => {
if (v === "all") return "all";
const match = options.find(({ id }) => String(id) === String(v));
return match ? match.id : v;
});
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/components/mui/dropdown-checkbox.js` around lines 36 - 41, Normalize the
types of items in selected when converting ev.target.value so downstream
comparisons like selected.includes(id) and value.includes(id) match the option
id types: after splitting the string fallback, map the selected array to the
same type as your option ids (e.g. detect typeof options[0].id — if it's
"number" convert numeric-looking strings with Number(...) or parseInt, otherwise
leave strings) so comparisons against id work correctly; update the code that
builds selected from ev.target.value (the rawValue -> selected conversion) to
perform this normalization.


if (selected.includes("all")) {
if (!value.includes("all")) {
Expand All @@ -43,6 +49,8 @@ const DropdownCheckbox = ({
onChange({
target: { name, value: selected.filter((v) => v !== "all") }
});
} else {
onChange({ target: { name, value: ["all"] } });
}
} else {
// else if "all" is not selected we just send selection
Expand All @@ -59,6 +67,7 @@ const DropdownCheckbox = ({
multiple
value={value}
onChange={handleChange}
{...rest}
input={<OutlinedInput label={label} />}
renderValue={(selected) => {
if (selected.includes("all")) {
Expand Down
12 changes: 12 additions & 0 deletions src/utils/methods.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,3 +297,15 @@ export const convertSVGtoImg = async (svgUrl) => {

export const isRateEnabled = (value) =>
value !== null && value !== undefined && value !== "";

/**
* Returns true if value is null, undefined, empty/whitespace string,
* empty array, or empty object.
*/
export const empty = (value) => {
if (value === null || value === undefined) return true;
if (typeof value === "string") return value.trim().length === 0;
if (Array.isArray(value)) return value.length === 0;
if (typeof value === "object") return Object.keys(value).length === 0;
return false;
};
Loading