Skip to content
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

Remove fontawesome dependency #15133

Merged
merged 13 commits into from Dec 21, 2022
Merged

Remove fontawesome dependency #15133

merged 13 commits into from Dec 21, 2022

Conversation

gu-stav
Copy link
Contributor

@gu-stav gu-stav commented Dec 8, 2022

What does it do?

Removes fontawesome from components, to reduce the bundle size:

  • components share the same icon: it is no longer possible to select a specific icon for a component
  • auth providers no longer have an icon

Why is it needed?

It reduces the bundle-size from 9.97 MB to 8.01 MB.

TODO

  • Add question mark icon to DS and fix onboarding

How to test it?

Views using FA icons:

  • CTB: /admin/plugins/content-type-builder/content-types/api::restaurant.restaurant
  • CM Edit View: /admin/content-manager/collectionType/api::kitchensink.kitchensink/create
  • CM Configure The View: /admin/content-manager/collectionType/api::kitchensink.kitchensink/configurations/edit
  • Providers /admin/settings/users-permissions/providers

Related issue(s)/PR(s)

Fixes #14572

@gu-stav gu-stav added source: core:admin Source is core/admin package pr: chore This PR contains chore tasks (cleanups, configs, tooling...) labels Dec 8, 2022
@gu-stav gu-stav force-pushed the chore/remove-fortawesome branch 5 times, most recently from c521e4b to 88ed9bf Compare December 8, 2022 20:24
@gu-stav gu-stav force-pushed the chore/remove-fortawesome branch 2 times, most recently from b5fb74c to 1ca2c10 Compare December 20, 2022 13:53
@gu-stav gu-stav marked this pull request as ready for review December 20, 2022 14:09
@codecov
Copy link

codecov bot commented Dec 20, 2022

Codecov Report

Base: 60.34% // Head: 60.52% // Increases project coverage by +0.18% 🎉

Coverage data is based on head (a486963) compared to base (76bdf07).
Patch coverage: 95.31% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #15133      +/-   ##
==========================================
+ Coverage   60.34%   60.52%   +0.18%     
==========================================
  Files        1367     1352      -15     
  Lines       33268    33172      -96     
  Branches     6352     6334      -18     
==========================================
+ Hits        20076    20078       +2     
+ Misses      11347    11263      -84     
+ Partials     1845     1831      -14     
Flag Coverage Δ
back 50.36% <100.00%> (-0.02%) ⬇️
front 65.09% <95.23%> (+0.28%) ⬆️
unit_back 50.36% <100.00%> (-0.02%) ⬇️
unit_front 65.09% <95.23%> (+0.28%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...re/admin/admin/src/components/GlobalStyle/index.js 83.33% <ø> (-5.56%) ⬇️
.../admin/src/components/DataManagerProvider/index.js 0.00% <ø> (ø)
...r/admin/src/components/FormModal/component/form.js 33.33% <ø> (ø)
...pe-builder/admin/src/components/FormModal/index.js 0.00% <ø> (ø)
.../content-type-builder/admin/src/pages/App/index.js 0.00% <ø> (ø)
...pe-builder/server/controllers/validation/common.js 63.88% <ø> (-2.78%) ⬇️
...erver/services/schema-builder/component-builder.js 14.03% <ø> (ø)
...ers-permissions/admin/src/pages/Providers/index.js 69.66% <ø> (-0.34%) ⬇️
...ges/EditSettingsView/components/DynamicZoneList.js 62.50% <50.00%> (-15.28%) ⬇️
...-manager/components/ComponentIcon/ComponentIcon.js 100.00% <100.00%> (ø)
... and 9 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@joshuaellis joshuaellis left a comment

Choose a reason for hiding this comment

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

Few Q's nothing major

Copy link
Contributor

@ronronscelestes ronronscelestes left a comment

Choose a reason for hiding this comment

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

niiice - thank you for this, I checked every spot impacted locally and didn't run into any issues! 💃

Copy link
Contributor

@ronronscelestes ronronscelestes left a comment

Choose a reason for hiding this comment

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

LGTM 💃

@gu-stav gu-stav removed the flag: documentation This PR requires a documentation update label Dec 21, 2022
@gu-stav gu-stav dismissed joshuaellis’s stale review December 21, 2022 14:56

Requested changes have been addressed.

@gu-stav gu-stav merged commit 8d51c78 into main Dec 21, 2022
@gu-stav gu-stav deleted the chore/remove-fortawesome branch December 21, 2022 14:56
@gu-stav gu-stav modified the milestone: 4.5.6 Jan 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: chore This PR contains chore tasks (cleanups, configs, tooling...) source: core:admin Source is core/admin package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFC] Consider removing font-awesome
3 participants