search

plone / plone.staticresources

Static resources for Plone
https://pypi.org/project/plone.staticresources/
5 stars 13 forks source link

Move fonts to extra bundle #131

Closed agitator closed 3 years ago

agitator commented 3 years ago

Reduce bundle sizes by not inlining fonts in each bundle - moved plone-fontello and glyphicons to their own bundle.

Depends on: https://github.com/plone/mockup/pull/1042

This PR adds two new bundles: plone-fontello and plone-glyphicons.

The total CSS size is reduced about 700kB.

CSS bundle sizes before this PR:

224K    filemanager-compiled.css
168K    plone-compiled.css
16K plone-datatables-compiled.css
232K    plone-editor-tools-compiled.css
4.0K    plone-legacy-compiled.css
268K    plone-logged-in-compiled.css
376K    plone-tinymce-compiled.css
204K    resourceregistry-compiled.css
248K    thememapper-compiled.css
4.0K    tinymce-styles.css
1.8M    total

CSS bundle sizes with this PR:

104K    filemanager-compiled.css
20K plone-compiled.css
16K plone-datatables-compiled.css
116K    plone-editor-tools-compiled.css
32K plone-fontello-compiled.css
120K    plone-glyphicons-compiled.css
4.0K    plone-legacy-compiled.css
148K    plone-logged-in-compiled.css
256K    plone-tinymce-compiled.css
84K resourceregistry-compiled.css
132K    thememapper-compiled.css
4.0K    tinymce-styles.css
1.1M    total
mister-roboto commented 3 years ago

@agitator thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

agitator commented 3 years ago

@jenkins-plone-org please run jobs

agitator commented 3 years ago

@jenkins-plone-org please run jobs

thet commented 3 years ago

@jenkins-plone-org please run jobs

agitator commented 3 years ago

@mauritsvanrees should we try to remove the inlining of web fonts? The way it works now is that about 4 types of web fonts are included in the file rather than the one a specific browser needs...

https://github.com/plone/plone.staticresources/blob/move-fonts-to-extra-bundle/src/plone/staticresources/static/plone-glyphicons-compiled.css

I'd rather see specific urls to the actual fonts

@font-face {
  font-family: "Glyphicons Halflings";
  src: url("@{icon-font-path}@{icon-font-name}.eot");
  src: url("@{icon-font-path}@{icon-font-name}.eot?#iefix") format("embedded-opentype"),
       url("@{icon-font-path}@{icon-font-name}.woff2") format("woff2"),
       url("@{icon-font-path}@{icon-font-name}.woff") format("woff"),
       url("@{icon-font-path}@{icon-font-name}.ttf") format("truetype"),
       url("@{icon-font-path}@{icon-font-name}.svg#@{icon-font-svg-id}") format("svg");
}
agitator commented 3 years ago

@jenkins-plone-org please run jobs

mauritsvanrees commented 3 years ago

@agitator wrote:

@mauritsvanrees should we try to remove the inlining of web fonts?

I have no opinion on this. I think you already did that in a later commit, right?

In total, LGTM, though I did not try it out.

Do add-on developers need to change something because of this PR?

agitator commented 3 years ago

the changes shouldn't have an impact for addons @mauritsvanrees could you do a release?

mauritsvanrees commented 3 years ago

Yes, I can. I have added mockup and staticresources to the checkouts first.

agitator commented 3 years ago

wait with releasing, maybe found something...