-
Notifications
You must be signed in to change notification settings - Fork 246
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
fix: fe/refactor icon box #1435
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 | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,77 @@ | ||||||||||||||||
import { Box, styled } from "@mui/material"; | ||||||||||||||||
import PropTypes from "prop-types"; | ||||||||||||||||
|
||||||||||||||||
/** | ||||||||||||||||
* IconBox - A styled box component for rendering icons with consistent sizing and styling | ||||||||||||||||
* | ||||||||||||||||
* @component | ||||||||||||||||
* @param {Object} [props] - Configuration options for the IconBox | ||||||||||||||||
* @param {number} [props.height=34] - Height of the icon box | ||||||||||||||||
* @param {number} [props.width=34] - Width of the icon box | ||||||||||||||||
* @param {number} [props.minWidth=34] - Minimum width of the icon box | ||||||||||||||||
* @param {number} [props.borderRadius=4] - Border radius of the icon box | ||||||||||||||||
* @param {number} [props.svgWidth=20] - Width of the SVG icon | ||||||||||||||||
* @param {number} [props.svgHeight=20] - Height of the SVG icon | ||||||||||||||||
* | ||||||||||||||||
* @example | ||||||||||||||||
* // Basic usage | ||||||||||||||||
* <IconBox> | ||||||||||||||||
* <SomeIcon /> | ||||||||||||||||
* </IconBox> | ||||||||||||||||
* | ||||||||||||||||
* @example | ||||||||||||||||
* // Customized usage | ||||||||||||||||
* <IconBox | ||||||||||||||||
* height={40} | ||||||||||||||||
* width={40} | ||||||||||||||||
* svgWidth={24} | ||||||||||||||||
* svgHeight={24} | ||||||||||||||||
* > | ||||||||||||||||
* <CustomIcon /> | ||||||||||||||||
* </IconBox> | ||||||||||||||||
* | ||||||||||||||||
* @returns {React.ReactElement} A styled box containing an icon | ||||||||||||||||
*/ | ||||||||||||||||
const IconBox = styled(Box)( | ||||||||||||||||
({ | ||||||||||||||||
theme, | ||||||||||||||||
height = 34, | ||||||||||||||||
width = 34, | ||||||||||||||||
minWidth = 34, | ||||||||||||||||
borderRadius = 4, | ||||||||||||||||
svgWidth = 20, | ||||||||||||||||
svgHeight = 20, | ||||||||||||||||
}) => ({ | ||||||||||||||||
height: height, | ||||||||||||||||
minWidth: minWidth, | ||||||||||||||||
width: width, | ||||||||||||||||
position: "relative", | ||||||||||||||||
border: 1, | ||||||||||||||||
borderStyle: "solid", | ||||||||||||||||
borderColor: theme.palette.border.dark, | ||||||||||||||||
borderRadius: borderRadius, | ||||||||||||||||
backgroundColor: theme.palette.background.accent, | ||||||||||||||||
"& svg": { | ||||||||||||||||
position: "absolute", | ||||||||||||||||
top: "50%", | ||||||||||||||||
left: "50%", | ||||||||||||||||
transform: "translate(-50%, -50%)", | ||||||||||||||||
width: svgWidth, | ||||||||||||||||
height: svgHeight, | ||||||||||||||||
"& path": { | ||||||||||||||||
stroke: theme.palette.text.tertiary, | ||||||||||||||||
}, | ||||||||||||||||
Comment on lines
+61
to
+63
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. 🛠️ Refactor suggestion Mom's spaghetti moment: SVG path styling might break with some icons! The current implementation assumes all SVG icons use - "& path": {
- stroke: theme.palette.text.tertiary,
- },
+ "& path, & rect, & circle": {
+ stroke: theme.palette.text.tertiary,
+ fill: "none",
+ }, 📝 Committable suggestion
Suggested change
|
||||||||||||||||
}, | ||||||||||||||||
}) | ||||||||||||||||
); | ||||||||||||||||
|
||||||||||||||||
IconBox.propTypes = { | ||||||||||||||||
height: PropTypes.number, | ||||||||||||||||
width: PropTypes.number, | ||||||||||||||||
minWidth: PropTypes.number, | ||||||||||||||||
borderRadius: PropTypes.number, | ||||||||||||||||
svgWidth: PropTypes.number, | ||||||||||||||||
svgHeight: PropTypes.number, | ||||||||||||||||
}; | ||||||||||||||||
|
||||||||||||||||
export default IconBox; |
This file was deleted.
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.
🛠️ Refactor suggestion
Yo dawg, that border prop looks a bit sus!
The border property is set to
1
without a unit, which might cause inconsistencies across browsers. Let's make it explicit.📝 Committable suggestion