fix(icons): apply fill to AddCircleIcon path, drop full-canvas rect#1411
Merged
yi-nuo426 merged 2 commits intolayer5io:masterfrom Apr 17, 2026
Merged
Conversation
AddCircleIcon's SVG put fill={fill} (currentColor by default) on a
full-canvas 48x48 rect and left the actual icon path unfilled, so the
icon rendered as a solid rectangle of text color with a black
plus-in-circle on top — instead of a clean outlined-plus-in-circle
glyph.
Drop the background rect and apply fill={fill} to the icon path, so
AddCircleIcon renders like the @mui/icons-material/AddCircleOutline
it's intended to replace in consumers (e.g. meshery/ui).
Signed-off-by: Lee Calcote <lee.calcote@layer5.io>
There was a problem hiding this comment.
Code Review
This pull request simplifies the AddCircleIcon SVG by removing an unnecessary background path and applying the fill property directly to the icon path. A review comment suggests renaming the component AddIconCircleBordered to match the filename AddCircleIcon.tsx for better consistency and maintainability.
Signed-off-by: Yī nuò <218099172+yi-nuo426@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AddCircleIcon's SVG putfill={fill}(currentColorby default) on a full-canvas 48×48 rect and left the actual icon path unfilled. As a result the icon rendered as a solid rectangle of text color with a black plus-in-circle outline on top (SVG default-fill behavior), rather than a clean outlined-plus-in-circle glyph.This drops the background rect and applies
fill={fill}to the icon path, soAddCircleIconrenders like the@mui/icons-material/AddCircleOutlineit's meant to replace in consumers.Before / after SVG
The icon
dattribute is unchanged; only the paint instructions moved.Context
This is exposed by meshery/meshery#18796, which swaps
@mui/icons-material/AddCircleOutline→AddCircleIconfrom@sistent/sistent. Without this fix, that swap produces a visible regression (solid-colored rectangle with black outline instead of a clean outline).AddCircleIconwasn't used anywhere in meshery/ui before #18796, which is why this bug went unnoticed.Test plan
npm test— 7 suites, 21 tests all pass.<AddCircleIcon />renders matching@mui/icons-material/AddCircleOutlineon both light and dark themes.