[Icons] Add AppRegistrationIcon component#1492
[Icons] Add AppRegistrationIcon component#1492Rajesh-Nagarajan-11 merged 3 commits intolayer5io:masterfrom
Conversation
Signed-off-by: aprv10 <1apoorvindia@gmail.com>
There was a problem hiding this comment.
Code Review
This pull request introduces the AppRegistrationIcon component and its local export file. Feedback was provided to improve consistency by using the DEFAULT_FILL_NONE constant and to fix a potential bug where spreading props could overwrite the fill attribute with an undefined value. Additionally, the reviewer noted that the new icon should be exported from the central icons entry point.
Signed-off-by: aprv10 <1apoorvindia@gmail.com>
|
@aprv10 can you provide a recording with the dummy implementation of app-registration-icon at meshery-ui? I think it will be helpful to visualise |
|
@suzy-g38 I’ve been trying to get the UI working for quite some time now. I’ve set up the codebase locally but I still don’t see the icon reflected in the meshery UI. If possible, could you guide me on how I can properly test the AppRegistrationIcon implementation locally? That would be really helpful. From my side the implementation seems correct so I think I might be missing something in the setup or integration step. |
There was a problem hiding this comment.
@aprv10, so many changes have been made to this file. Please revert and only add the relevant changes.
There was a problem hiding this comment.
Most of the changes are from the linting fixes applied during the build process. The lines were mainly reformatted/moved around, so there’s effectively no functional change in that file apart from the export I added.
I’ll keep this in mind going forward and avoid including unrelated formatting changes.
Rajesh-Nagarajan-11
left a comment
There was a problem hiding this comment.
LGTM , Cross verified path with mui-icon

Notes for Reviewers
This PR fixes #1487
Signed commits