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
Add icon picker to components and show component's icons #16703
Conversation
This PR makes sense. But would it not be better to implement some kind of hook trough what you can add icons to your strapi instance. What would allow user to use font awesome or any other svg they want. Since now you are basically locking them in to only use your icons even when that does not make sense. |
Size Change: +2.97 kB (0%) Total Size: 1.5 MB
ℹ️ View Unchanged
|
packages/core/content-type-builder/admin/src/components/IconPicker/index.js
Outdated
Show resolved
Hide resolved
packages/core/content-type-builder/admin/src/components/IconPicker/index.js
Show resolved
Hide resolved
.../core/content-type-builder/admin/src/components/ComponentCard/ComponentIcon/ComponentIcon.js
Outdated
Show resolved
Hide resolved
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.
packages/core/admin/admin/src/content-manager/components/ComponentIcon/ComponentIcon.js
Outdated
Show resolved
Hide resolved
.../core/content-type-builder/admin/src/components/ComponentCard/ComponentIcon/ComponentIcon.js
Outdated
Show resolved
Hide resolved
packages/core/content-type-builder/admin/src/translations/en.json
Outdated
Show resolved
Hide resolved
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.
It's a good start :) Could you please add meaningful integration and unit tests wherever you can? Since this will be an important part of the component creation I think we should have a good coverage.
How are going to proceed in terms of documentation? It would be nice to add bits about it in the contributor docs and definitely in the user / dev docs.
...es/core/admin/admin/src/content-manager/pages/EditSettingsView/components/DynamicZoneList.js
Outdated
Show resolved
Hide resolved
packages/core/content-type-builder/admin/src/components/IconPicker/index.js
Outdated
Show resolved
Hide resolved
packages/core/content-type-builder/admin/src/components/IconPicker/index.js
Outdated
Show resolved
Hide resolved
packages/core/content-type-builder/admin/src/components/IconPicker/index.js
Outdated
Show resolved
Hide resolved
packages/core/content-type-builder/admin/src/components/IconPicker/index.js
Outdated
Show resolved
Hide resolved
packages/core/content-type-builder/admin/src/components/IconPicker/index.js
Outdated
Show resolved
Hide resolved
packages/core/content-type-builder/admin/src/components/IconPicker/index.js
Outdated
Show resolved
Hide resolved
packages/core/content-type-builder/admin/src/components/IconPicker/index.js
Outdated
Show resolved
Hide resolved
packages/core/content-type-builder/admin/src/components/IconPicker/index.js
Outdated
Show resolved
Hide resolved
Are these icons optional or required? |
@derrickmehaffy They are optional, if you don't select one it would show the cube icon by default like right now |
What would happen if you guys deleted one of your icons would that mean if you have that icon it crashes or would it just fall back to the default? Nvm looked at it again it it should just go to default then |
🥰 |
56d796a
to
ce18e78
Compare
...es/core/admin/admin/src/content-manager/pages/EditSettingsView/components/DynamicZoneList.js
Outdated
Show resolved
Hide resolved
.../core/content-type-builder/admin/src/components/ComponentCard/ComponentIcon/ComponentIcon.js
Outdated
Show resolved
Hide resolved
packages/core/content-type-builder/admin/src/components/ComponentCard/index.js
Outdated
Show resolved
Hide resolved
packages/core/content-type-builder/admin/src/components/IconPicker/index.js
Outdated
Show resolved
Hide resolved
packages/core/content-type-builder/admin/src/components/IconPicker/index.js
Outdated
Show resolved
Hide resolved
packages/core/content-type-builder/admin/src/components/IconPicker/tests/index.test.js
Outdated
Show resolved
Hide resolved
c0e6eef
to
463e245
Compare
.../core/content-type-builder/admin/src/components/ComponentCard/ComponentIcon/ComponentIcon.js
Outdated
Show resolved
Hide resolved
.../core/content-type-builder/admin/src/components/ComponentCard/ComponentIcon/ComponentIcon.js
Outdated
Show resolved
Hide resolved
packages/core/content-type-builder/admin/src/components/IconPicker/index.js
Show resolved
Hide resolved
packages/core/content-type-builder/admin/src/components/IconPicker/index.js
Outdated
Show resolved
Hide resolved
packages/core/content-type-builder/admin/src/components/IconPicker/index.js
Outdated
Show resolved
Hide resolved
382a255
to
66155f6
Compare
…gsView/components/DynamicZoneList.js Co-authored-by: Josh <37798644+joshuaellis@users.noreply.github.com>
b4a4f22
to
420f300
Compare
You can only have one |
Hey @joshuaellis yeah the documentation part is already covered and we don't need a migration guide considering how we ended handle this. I dismissed your review because I needed to make a rebase and solve some conflicts, sorry for ask for the review again and thanks for the approved again haha |
What does it do?
Last year we removed FontAwesome dependency on #15133 , on that PR we remove the icons from components on Content Manager and CTB because they were using FontAwesome. With this PR we add again the icon picker on components as well as the icon when you are creating a new entry with a Dynamic Zone, but this time using our own components from the Design System and not FontAwesome
How to test it?