search

readium / readium-css

🌈 A set of reference stylesheets for EPUB Reading Systems, starting with Readium Mobile
https://readium.org/readium-css/
BSD 3-Clause "New" or "Revised" License
92 stars 20 forks source link

Minify dist stylesheets #90

Closed JayPanoz closed 5 months ago

JayPanoz commented 4 years ago

I'm submitting a feature request

Short description of the issue/suggestion:

We’ve kept dist stylesheets DEV-ish (comments, etc.) so far but we should probably minify dist stylesheets given they may be injected in every EPUB file, etc. If we output map files, dev needs should be covered.

Moreover, minifying CSS is quite a strong recommandation for web perf.

See postcss-clean

Re. #89

danielweck commented 4 years ago

Suggestion: if CSS sourcemaps remain git-ignored, then please remove the references in the output "dist" stylesheets, e.g.:

https://github.com/readium/readium-css/blob/583011453612e6f695056ab6c086a2c4f4cac9c0/css/dist/ReadiumCSS-after.css#L923

Otherwise - if sourcemaps are not git-ignored anymore - then of course please leave the references in the output "dist" stylesheets, and consumer apps can decide whether to include them of not in their deployed app bundles.

PS: as discussed in the conference call, as a developer who does not need to trace ReadiumCSS issues back to the original SASS/other authoring format, I find that the Chromium "web inspector" provides excellent debugging tools to explore the CSS cascade and to understand/tweak the CSS selector specificity of app-level or navigator-level styling rules. In other words, I can see how a ReadiumCSS maintainer would find sourcemaps very useful, but the practical case for an app/SDK developer is less obvious, I think.

JayPanoz commented 4 years ago

Thanks to Daniel for recap’ing our discussion during the Readium Call. New version is available in the develop branch, with highlights and references to source maps removed. In addition, minification of dist stylesheets has been put in place.

So this will be automatically closed once develop is merged into master.

JayPanoz commented 5 months ago

Closing, as the topic is now discussed in #132