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

👌 IMPROVE: Move from CSS to SCSS #31

Merged
merged 5 commits into from
Sep 4, 2020
Merged

👌 IMPROVE: Move from CSS to SCSS #31

merged 5 commits into from
Sep 4, 2020

Conversation

chrisjsewell
Copy link
Member

@chrisjsewell chrisjsewell commented Sep 3, 2020

The goals were:

  1. Good common CSS practices: using (linted) SASS, compressing compiled CSS, using hashes in filenames (and ideally also using source maps)
  2. Allowing for "interactive development", with live rebuilds, while editing "watched" source files' both content files (i.e. .md,.rst,etc) and style files (i.e. .scss)

sphinx-panels does not contain any JS, and so I have not directly considered that.

The changes were:

  1. SCSS compiling is internal to the sphinx-build, using pyscss. As we noted in ⬆️ UPGRADE: Use pyScss instead of libsass sphinx-book-theme#200, libsass is the more "complete" compiler but it is quite "heavy-weight" for a dependency, requiring C-extensions. There is currently an annoying FutureWarning with pyscss that appears in build stdout, but the fix has been merged and hopefully should be released soon. pyscss does also currently preclude the addition of source maps 😢
  2. The CSS filename contains the content hash, to invalidate caches (closing CSS updates are not visible sometimes in a browser sphinx-book-theme#204)
  3. The CSS is written into the build folder, rather than a folder in the package directory (as is currently done in sphinx-book-theme)
  4. All documents are re-written on CSS changes, this supports...
  5. A tox.ini is added that includes a live rebuild environment, using sphinx-autobuild, that now properly works! (i.e. css is updated in the build folder) Given this works in tox, I see no need for nox (as currently used in sphinx-book-theme).
  6. Adds an scss linter in the pre-commit

The other option considered was to pre-compile the scss to css with a pre-commit hook (to ensure the css is kept consistent).
This negates the requirement for a sass compiler install dependency.
But I was having a look at this in https://github.com/executablebooks/pre-commit-scss, and its not trivial (dealing with git) and would be more difficult to get working with sphinx-autobuild

A note on to other implementation re CSS, for future discussion:

  1. Sphinx offers a templating functionality, whereby css_t are first processed by jinja. This allows for configuration based CSS, e.g. used in insipid-sphinx-theme. It's an interesting use case, however I don't like that this then prohibits good CSS development practices like those used above. If desired, one could think about confining such configuration to a single SCSS fragment, which is written dynamically (to override a default) before compilation.

  2. pydata-sphinx-theme uses webpack to pre-compile JS/CSS (see its guide). This is a more mature standard, but the drawback is that it adds a larger development dependency overhead, and does not fit in well (at least currently) with sphinx-autobuild "live development" or allow for configurational CSS.
    Edit: there is a live rebuild feature using yarn through: webpack.dev.js, but yeh it just adds a lot of overhead for a purely python package 🤔

The SCSS is compiled within the sphinx build, and the CSS is output filename includes the hash, to ensure cache invalidation on updates.
Lastly, all files are rewritten, if a CSS hash changes, to allow for sphinx-autobuild to work correctly (in tox.ini)
@chrisjsewell
Copy link
Member Author

chrisjsewell commented Sep 3, 2020

@choldgraf @pradyunsg @AakashGfude you might want to have a look at this, as it's a prelude to changing sphinx-book-theme in the same way.

@chrisjsewell chrisjsewell changed the title 👌 IMPROVE: Move CSS to SCSS 👌 IMPROVE: Move from CSS to SCSS Sep 3, 2020
@chrisjsewell
Copy link
Member Author

Also my sass game has upped a few levels, looking through the bootstrap repo 🤓

@pradyunsg
Copy link
Member

pradyunsg commented Sep 3, 2020

My suggestion would be to forfeit keeping master installable via git, omitting the generated file from the repo and only adding the "original" sources here.

That's also what I'm doing in https://pradyunsg.me/furo.

@chrisjsewell
Copy link
Member Author

My suggestion would be to forfeit keeping master installable via git, omitting the generated file from the repo and only adding the "original" sources here.

I'm not sure what you mean by this? None of the generated CSS is part of the repo, only the source SCSS

@AakashGfude
Copy link
Member

@chrisjsewell nice work. The hash encoding is also looking neat.

@pradyunsg
Copy link
Member

I'm not sure what you mean by this? None of the generated CSS is part of the repo, only the source SCSS

Ah, I meant:

  • source repository
    • no generated CSS
    • source SCSS files
  • distributions
    • generated CSS
    • no source SCSS files

Basically, there'd be a compile step between the source repository and the final distributions that get published on PyPI. I've opted for a Gulp-based pipeline over on the project I linked to (which lets me use popular/canonical implementations of the tools, instead of depending on ports).

@chrisjsewell
Copy link
Member Author

Ok, I'm going to merge this, to record the changes. But I might play around with pre-packaging with webpack, gulp, parcel, etc

@chrisjsewell chrisjsewell merged commit bcdbb2f into master Sep 4, 2020
@chrisjsewell chrisjsewell deleted the use-scsc branch September 4, 2020 19:58
chrisjsewell added a commit that referenced this pull request Sep 15, 2020
Following my musings in #31,
I've move away from the "dynamic" SCSS compilation (i.e. during sphinx-build), and instead implemented https://github.com/executablebooks/scss-compile.
This allows for a pre-commit compilation of the CSS, which ensures it is synced with the SCSS (and so both are present in the repo). This ticks the boxes:

- No installation dependency on an SCSS compiler, meaning also no potential issues with SCSS compilation for users.
- You don't have to implement an overly "heavy" web-bundler setup, like weppack or gulp
- It still works with tox + sphinx-autobuild, for live development that updates on code/SCSS/docs changes
- Hashed files, to ensure cache invalidation for new CSS
- Later possibility for CSS -> SCSS mapping implementation.

I've also added the (CSS only) `tabbed` directive 😄, and set up the docs/tox to build for multiple themes in the same branch (see the Development section of the README):

- With `sphinx-rtd-theme`: https://sphinx-panels--32.org.readthedocs.build/en/32/#tabbed-content
- With `sphinx-book-theme`: https://106-260360729-gh.circle-artifacts.com/0/html/index.html#tabbed-content

Lastly, this now also includes `conf.py` configurable CSS variables, *via* creation of a separate CSS file, that just contains some [CSS custom properties](https://developer.mozilla.org/en-US/docs/Web/CSS/Using_CSS_custom_properties). This allows for configuration of certain variables (like tab label color) without the need to create custom CSS overrides.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants