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

Minify dist stylesheets #90

Closed
JayPanoz opened this issue Jun 2, 2020 · 3 comments
Closed

Minify dist stylesheets #90

JayPanoz opened this issue Jun 2, 2020 · 3 comments
Labels
low-hanging fruit Issues you can tackle as your first code contribution

Comments

@JayPanoz
Copy link
Collaborator

JayPanoz commented Jun 2, 2020

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

@JayPanoz JayPanoz added the low-hanging fruit Issues you can tackle as your first code contribution label Jun 2, 2020
@danielweck
Copy link
Member

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

/*# sourceMappingURL=ReadiumCSS-after.css.map */

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 added a commit that referenced this issue Jun 3, 2020
JayPanoz added a commit that referenced this issue Jun 3, 2020
@JayPanoz
Copy link
Collaborator Author

JayPanoz commented Jun 3, 2020

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
Copy link
Collaborator Author

Closing, as the topic is now discussed in #132

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
low-hanging fruit Issues you can tackle as your first code contribution
Projects
None yet
Development

No branches or pull requests

2 participants