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

Switch to css imports rather than css_files hackery. #159

Merged
merged 5 commits into from
May 18, 2017

Conversation

ryan-roemer
Copy link
Owner

The theme uses css_files hackery, but that is deprecated in modern Sphinx and error prone because there are real object instances and not strings that now comprise css_files. See, e.g. #158

This PR switches all of the conditionally-included CSS files into css @import url("NAME.css"); statements and just now has one root style sheet included the old fashioned way. Hopefully this is both backwards compatible with any previous Sphinx versions and works better as css_files goes off into deprecation sunset.

Would appreciate folks pulling this down and kicking the tires!

/cc @alexhagen @has2k1 @pcav

@alexhagen
Copy link

Works with no warnings for my purposes with sphinx 1.6.2 and sphinx 1.6.1 with a default bootswatch Journal stylesheet.

However, the css override done in _templates/layout.html

{% set bootswatch_css_custom = ['_static/bootstrap.css'] %}

no longer overrides the stylesheet. Is there a new/different api for custom css overrides?

@has2k1
Copy link

has2k1 commented May 17, 2017

I have the same issue as @alexhagen

In _templates/layout.html this

{# Import the theme's layout. #}
{% extends "!layout.html" %}

{# Custom CSS overrides #}
{% set bootswatch_css_custom = ['_static/custom.css'] %}

does not include the custom css file.

@ryan-roemer
Copy link
Owner Author

ryan-roemer commented May 17, 2017

It should actually be:

{% set bootswatch_css_custom = ['custom.css'] %}

Without the _static because it's relative, but it's still not working in my demo because I think the CSS template processing happens before layout.html, so I'll need to come up with another approach.

This use case here is really "add CSS stylesheets at the end", so ideally I'll find something more idiomatic and sustainable than this...

@has2k1
Copy link

has2k1 commented May 17, 2017

What about the other bootstrap css files that are not included. Is it the same issue

These ones

<link rel="stylesheet" href="_static/bootstrap-3.3.6/css/bootstrap.min.css" type="text/css" />
<link rel="stylesheet" href="_static/bootstrap-3.3.6/css/bootstrap-theme.min.css" type="text/css" />

Though, I do not see any effect on the styling due them being absent.

@ryan-roemer
Copy link
Owner Author

Those should correctly be imported using the CSS style import now. I forget how it shows up in the dev tools, but if you look at your Web servers access log all the correct CSS files should be accessed.

I'm still working on the customization problem

  `#158 <https://github.com/ryan-roemer/sphinx-bootstrap-theme/pull/158>`_,
  `#160 <https://github.com/ryan-roemer/sphinx-bootstrap-theme/pull/160>`_.

* **Breaking Change**: Remove ``bootswatch_css_custom`` override, and instead opt for documenting idiomatic Sphinx-version specific generic overrides for custom CSS.
@ryan-roemer
Copy link
Owner Author

@alexhagen @has2k1 @pcav @jaraco -- OK, I've updated the documentation for doing custom CSS to make it (1) generic and (2) have documented strategies for Sphinx before and after 1.5 (since things are radically different).

See here: https://github.com/ryan-roemer/sphinx-bootstrap-theme/blob/0ecb1832f1ed53656392b702073e1d0513b8cbb6/README.rst#adding-custom-css

Let me know how everything looks for:

  • Rendering stuff in Sphinx 1.5 and 1.6
  • The same with custom CSS overrides via the newly documented solutions

Thanks!

@has2k1
Copy link

has2k1 commented May 17, 2017

@ryan-roemer, it works well. Added advantage is that now you add extra css the same way you add extra javascript.

@alexhagen
Copy link

@ryan-roemer I agree with @has2k1 , I like the API. Thanks again!

@ryan-roemer
Copy link
Owner Author

ryan-roemer pushed a commit that referenced this pull request Jul 23, 2017
- minor doc fixes
    - I believe I got all 3.3.6 into 3.3.7
    - suggestion to use bs3 over 2
- more specific directions on how to upgrade in future
    - missing documentation of `static/bootstrap-sphinx.css_t` for upgrades
    - much more description of what should go down in `TODO.rst`
        - modified the **epic** `tar` packaging to be more explicit (anybody who has `egrep` aliased can't use the original solution)
- fixed `Makefile` to actually use `python3`
    - added simple option for port override (I'm currently running a mission critical app on `8000` xD)
- fixed `requirements.txt` to allow e.g. `sphinx==1.6.2` or higher
    - `setup.py` has neither of these dependencies?
    - if you add them, you'll need to treat python 2 and 3 separately because of fabric?

#### Notes:

- Candidate to close #149
- Officially the `solar` theme is not in the `v3.3.7` bootswatch tag, see [this issue](thomaspark/bootswatch#643)
    - Just added it manually from the main page
- I don't think you actually want to include the `custom` theme, I'm pretty sure it's either an artifact or a basis for creating your own.
- Changed a few locations referring to the now nonexistent `amelia` theme
 
#### Questions:

1. There is a rather long commented out section in `TODO.rst`.  Might be worth deleting?
2. See changes to `README.rst`, in particular the added `.. warning::`.  The question: I didn't really understand what was going on with #159 because I didn't have any trouble building with `sphinx==1.5.1` or `1.5.5` etc.  Basically, if it's actually a hard requirement then maybe the verbiage is fine, but if not then it's too strong?
3. This is not a "you" problem, but when viewing the source of generated pages I noticed all of the javascript gets loaded at the beginning in the `<meta>` rather than at the bottom.  I know next to nothing about front-end, but those are supposed to be loaded last right (sphinx is doing this, not you)?

Happy to undo / revise any of the included, thanks again for the theme 😄
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.

None yet

3 participants