Skip to content

fix: match inputs with figma styles #261

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

Open
priscila-moneo wants to merge 2 commits into
base: main
Choose a base branch
from
+54 −1
Open

fix: match inputs with figma styles #261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
027c217
fix: match inputs with figma styles
priscila-moneo Jun 1, 2026
e81f567
feat: added custom theme
priscila-moneo Jun 3, 2026
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
5 changes: 4 additions & 1 deletion src/components/mui/search-input.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,10 @@ const SearchInput = ({ term, onSearch, placeholder = "Search...", debounced }) =
"& .MuiOutlinedInput-root": {
height: "36px",
paddingX: 1
},
},
"& .MuiOutlinedInput-root .MuiOutlinedInput-notchedOutline": { borderColor: "rgba(0, 0, 0, 0.23)" },
Copy link
Copy Markdown
Contributor

@santipalenque santipalenque Jun 4, 2026

Choose a reason for hiding this comment

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

@smarcet another example, we need to standarize these colors and use one in the color palette or use a custom one set on CustomTheme. If for some reason we don't want to do that because is too specific, then we should use one of the definitions gray[500] or something. In this particular case @priscila-moneo looks like this color is the default for border color on outline variant so I don't understand why are you overriding with that same color

Copy link
Copy Markdown
Contributor Author

@priscila-moneo priscila-moneo Jun 5, 2026

Choose a reason for hiding this comment

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

The override is intentional. The outlined variant default border color was changed to #000, but according to the current Figma designs, the Search input should keep using the previous border color instead of inheriting the new default. That's why the color is explicitly set here.
image
image

Copy link
Copy Markdown

@JpMaxMan JpMaxMan Jun 5, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Consistency is key in design systems. We should check with design team and see which one should be used and if it's differentiated by input type categorize that difference so it can be referenced the same way by all input types of that category.

"& .MuiOutlinedInput-root:hover .MuiOutlinedInput-notchedOutline": { borderColor: "rgba(0, 0, 0, 0.23)" },
"& .MuiOutlinedInput-root.Mui-focused .MuiOutlinedInput-notchedOutline": { borderColor: "primary.main" }
Comment on lines +87 to +89
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 3, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Border-state overrides conflict with the centralized theme contract.

At Line 87-89, SearchInput hardcodes notched-outline colors (rgba(...) and primary.main), but src/utils/theme.js already centralizes MuiOutlinedInput border colors for default/hover/focus (#000). This creates cross-component inconsistency and defeats the new theme abstraction.

Suggested fix 
       sx={{
         "& .MuiOutlinedInput-root": {
           height: "36px",
           paddingX: 1
-        },
-        "& .MuiOutlinedInput-root .MuiOutlinedInput-notchedOutline": { borderColor: "rgba(0, 0, 0, 0.23)" },
-        "& .MuiOutlinedInput-root:hover .MuiOutlinedInput-notchedOutline": { borderColor: "rgba(0, 0, 0, 0.23)" },
-        "& .MuiOutlinedInput-root.Mui-focused .MuiOutlinedInput-notchedOutline": { borderColor: "primary.main" }
+        }
       }}
📝 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
"& .MuiOutlinedInput-root .MuiOutlinedInput-notchedOutline": { borderColor: "rgba(0, 0, 0, 0.23)" },
"& .MuiOutlinedInput-root:hover .MuiOutlinedInput-notchedOutline": { borderColor: "rgba(0, 0, 0, 0.23)" },
"& .MuiOutlinedInput-root.Mui-focused .MuiOutlinedInput-notchedOutline": { borderColor: "primary.main" }
sx={{
"& .MuiOutlinedInput-root": {
height: "36px",
paddingX: 1
}
}}
🤖 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/search-input.js` around lines 87 - 89, SearchInput
contains hardcoded MuiOutlinedInput notchedOutline color overrides that conflict
with the centralized theme; remove the three style overrides using "&
.MuiOutlinedInput-root .MuiOutlinedInput-notchedOutline", "&
.MuiOutlinedInput-root:hover .MuiOutlinedInput-notchedOutline", and "&
.MuiOutlinedInput-root.Mui-focused .MuiOutlinedInput-notchedOutline" from the
SearchInput styles and let the global MuiOutlinedInput theme in theme.js control
default/hover/focus borders, or if a local variant is required read colors from
the theme (e.g., theme.palette.*) instead of hardcoded "rgba(...)" or
"primary.main" so the component honors the centralized contract.

}}
/>
);
Expand Down
49 changes: 49 additions & 0 deletions src/utils/theme.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/**
* Copyright 2026 OpenStack Foundation
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* */

import { createTheme } from "@mui/material/styles";
import KeyboardArrowDownIcon from "@mui/icons-material/KeyboardArrowDown";

const uiCoreThemeOverrides = {
components: {
MuiOutlinedInput: {
styleOverrides: {
notchedOutline: {
borderColor: "#000"
},
root: {
"&:hover .MuiOutlinedInput-notchedOutline": {
borderColor: "#000"
},
"&.Mui-focused .MuiOutlinedInput-notchedOutline": {
borderColor: "#000"
}
}
}
},
MuiSelect: {
defaultProps: {
IconComponent: KeyboardArrowDownIcon
},
styleOverrides: {
icon: {
fontSize: "24px",
width: "24px",
height: "24px"
}
}
}
}
};

export const CustomThemeBase = createTheme(uiCoreThemeOverrides);
Copy link
Copy Markdown
Contributor

@santipalenque santipalenque Jun 4, 2026

Choose a reason for hiding this comment

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

I don't think we need to createTheme in uicore, just export the definitions and let the client app use it as they need. Also please name it MuiBaseCustomTheme

1 change: 1 addition & 0 deletions webpack.common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ module.exports = {
'utils/money': './src/utils/money.js',
'utils/use-event-callback': './src/utils/use-event-callback.js',
'utils/external-store': './src/utils/external-store.js',
'utils/theme': './src/utils/theme.js',
},
output: {
path: path.resolve(__dirname, 'lib'),
Expand Down
Loading