Skip to content

Close #8634: html: Allow to change the order of JS/CSS#8639

Merged
tk0miya merged 1 commit intosphinx-doc:3.xfrom
tk0miya:8634_css_priority
Jan 2, 2021
Merged

Close #8634: html: Allow to change the order of JS/CSS#8639
tk0miya merged 1 commit intosphinx-doc:3.xfrom
tk0miya:8634_css_priority

Conversation

@tk0miya
Copy link
Member

@tk0miya tk0miya commented Jan 1, 2021

Feature or Bugfix

  • Feature

Purpose

`Sphinx.add_js_file()` and `Sphinx.add_css_file()` take `priority`
argument to change the order of JS/CSS files.
@tk0miya tk0miya added type:enhancement enhance or introduce a new feature builder:html labels Jan 1, 2021
@tk0miya tk0miya added this to the 3.5.0 milestone Jan 1, 2021
:widths: 20,80

* - Priority
- Main purpose in Sphinx
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: At present, Sphinx's CSS files are embeded into theme files directly. So it is hard to control its order. I'll improve it in another PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I posted #8643.

@choldgraf
Copy link
Contributor

I think this is great 👍

@akhmerov
Copy link

akhmerov commented Jan 2, 2021

I also think this is wonderful: especially when combined with #8634, it is a drastic quality of life improvement—thanks a lot!

I also like that without extensions and users doing anything this impementation guarantees that extension assets always come before the user assets, unlike in the previous implementation.

I have the following remarks for consideration:

  • This implementation preserves the implicit existing feature that the order of calls to add_js_file and add_css_file with equal priority is the same as the order of asset inclusion. This is guaranteed due to list.sort being stable. The same holds for the user-provided assets (html_js_files / html_css_files). Should this be made into an explicit guarantee and documented?
  • The feature seems to be easier to use for extension authors than for end users, since there's no interface for specifying priority in html_js_files and html_css_files. This seems to be quite alright: the advanced users may turn their conf.py into an extension.

@tk0miya tk0miya merged commit 1b7d165 into sphinx-doc:3.x Jan 2, 2021
@tk0miya tk0miya deleted the 8634_css_priority branch January 2, 2021 14:34
@tk0miya
Copy link
Member Author

tk0miya commented Jan 2, 2021

Oops. I overlooked @akhmerov 's last comment.

Should this be made into an explicit guarantee and documented?

Indeed. It would be better if documented.

The feature seems to be easier to use for extension authors than for end users, since there's no interface for specifying priority in html_js_files and html_css_files.

I did not think about the user configurations. But it is also allowed to give priority via attribute with this change:

html_css_files = [('custom.css', {'priority': 999}]

@tk0miya
Copy link
Member Author

tk0miya commented Jan 2, 2021

I posted #8641 for additional documentation. It would be helpful if you'll review it also (because I'm not good at English...)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

builder:html type:enhancement enhance or introduce a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants