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

RFE: Reduce project's JS/CSS payload size #87

Closed
pradyunsg opened this issue Aug 24, 2020 · 15 comments · Fixed by #94
Closed

RFE: Reduce project's JS/CSS payload size #87

pradyunsg opened this issue Aug 24, 2020 · 15 comments · Fixed by #94

Comments

@pradyunsg
Copy link
Member

Is your feature request related to a problem? Please describe.

Currently, this extension results in 50+ kB of assets across 7+ network requests to be added to each page for a project's documentation.

Describe the solution you'd like

Significantly lowered payload size (drop the dependency on semantic-ui?) and consolidate CSS/JS files to reduce the number of network requests needed.

Describe alternatives you've considered

A clear and concise description of any alternative solutions or features you've considered.

Additional context

This would also make it easier for "downstream" themes to customize how the tabs look, if the CSS for this project is self-contained in a single file. :)

@welcome
Copy link

welcome bot commented Aug 24, 2020

Thanks for opening your first issue here! Engagement like this is essential for open source projects! 🤗

If you haven't done so already, check out EBP's Code of Conduct. Also, please try to follow the issue template as it helps other community members to contribute more effectively.

If your issue is a feature request, others may react to it, to raise its prominence (see Feature Voting).

Welcome to the EBP community! 🎉

@chrisjsewell
Copy link
Member

Also, I've recently realised that its not too hard to add js only for specific pages, i.e. those containing tab nodes (see executablebooks/sphinx-book-theme#165 (comment)).
So I think this would be a complimentary improvement

@pradyunsg
Copy link
Member Author

Would you be open to a complete rewrite of this package, similar to what I did for sphinx-autobuild? 👀

@chrisjsewell
Copy link
Member

Not really lol, I've already set it up consistent with the EBP guidelines in #76

Obviously I'm open to suggestions and improvements where beneficial.
But I will never agree to using a src folder 🤣

@Daltz333
Copy link
Collaborator

src for python is just weird....

@chrisjsewell
Copy link
Member

src for python is just weird....

FYI, I mainly said this to "pull @pradyunsg's leg", since I know he is a bit of a fan pypa/packaging.python.org#320 lol

@pradyunsg
Copy link
Member Author

Obviously I'm open to suggestions and improvements where beneficial.

Noxify!

@chrisjsewell
Copy link
Member

Noxify!

Well there already a tox.ini.
I'd rather just use that for 2 reasons

  1. tox is just more mature and well-used/known
  2. I personally use also the tox-conda plugin which is super useful, and I don't see a nox equivalent (apart from hard-coding conda into nox.py itself)

nox has its uses; the live-rebuilds for messing around with scss in sphinx-book-theme is helpful.
But I'm not sure, for a project less based around theming, that the pros outweigh the cons?

@pradyunsg
Copy link
Member Author

pradyunsg commented Aug 30, 2020

Fair enough. :)

As long as my finger-memory of `.ox -s (tests?|lint|docs) works, I'm a happy kiddo. :)

@chrisjsewell
Copy link
Member

nox -s lint was actually a nitpick I noted in sphinx-book-theme/autobuild.
Surely its redundant to create a virtualenv, in which to run pre-commit, which then creates its own virtualenv 😜

@pradyunsg
Copy link
Member Author

pradyunsg commented Aug 30, 2020

It is super redundant. OTOH, the main reason I do this is consistency across projects (not all my projects use pre-commit, yet!) and newbie-friendliness. It's easier to tell newcomers to "install nox, and use these nox commands" than "install nox and pre-commit, and use pre-commit for linting, everything else uses these nox commands".

And for anyone familiar enough with pre-commit, well, pre-commit run --all-files still works. :)

@chrisjsewell
Copy link
Member

linking executablebooks/meta#119

@pradyunsg
Copy link
Member Author

pradyunsg commented Sep 14, 2020

@chrisjsewell Thoughts on a complete rewrite of this project, to drop the dependency on semantic-ui + works with CSS-only w/ JS for "advanced" features like grouped tabs? It should make it easier to tweak the JS to implement stuff like #84 and #47.

@chrisjsewell
Copy link
Member

I'd say possibly, but it's low on the priority list.
Obviously we now have, https://sphinx-panels.readthedocs.io/en/latest/#tabbed-content for a CSS only approach,
and first I'd like to get executablebooks/meta#137 sussed

@pradyunsg
Copy link
Member Author

This RFE is still valid, and @foster999 has made #94, which looks like it does the needful. I'm not gonna come around to this myself anytime soon though. Context here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants