executablebooks / sphinx-tabs

Tabbed views for Sphinx
https://sphinx-tabs.readthedocs.io
MIT License
264 stars 68 forks source link

πŸ‘Œ IMPROVE: Add sphinx option to disable css from loading #117

Closed dgarcia360 closed 3 years ago

dgarcia360 commented 3 years ago

Closes https://github.com/executablebooks/sphinx-tabs/issues/115

How to test this PR

To disable the CSS from loading, add the following to your conf.py:

sphinx_tabs_disable_css_loading = True

Once you build the docs, you should see all tabs unstyled:

image

welcome[bot] commented 3 years ago

Thanks for submitting your first pull request! You are awesome! :hugs:
If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.
Welcome to the EBP community! :tada:

codecov[bot] commented 3 years ago

Codecov Report

Merging #117 (09f0f3a) into master (45308dd) will increase coverage by 0.03%. The diff coverage is 83.33%.

:exclamation: Current head 09f0f3a differs from pull request most recent head 5c38367. Consider uploading reports for the commit 5c38367 to get more accurate results Impacted file tree graph

@@            Coverage Diff             @@
##           master     #117      +/-   ##
==========================================
+ Coverage   97.16%   97.20%   +0.03%     
==========================================
  Files           2        2              
  Lines         212      215       +3     
==========================================
+ Hits          206      209       +3     
  Misses          6        6              
Flag Coverage Ξ”
pytests 97.20% <83.33%> (+0.03%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Ξ”
sphinx_tabs/tabs.py 97.18% <83.33%> (+0.04%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Ξ” = absolute <relative> (impact), ΓΈ = not affected, ? = missing data Powered by Codecov. Last update 45308dd...5c38367. Read the comment docs.

foster999 commented 3 years ago

Thanks for the improvement @dgarcia360!

It would be great to have an additional test, to assert that the CSS isn't loaded. I'm happy to help, or add this myself?

I'm confused that this change is causing the JS tests to fail - hopefully fixing the formatting for pre-commit will make that disappear too...

foster999 commented 3 years ago

Just checked and the JS test failure seems to occur on any new PR, so shall look into this.

foster999 commented 3 years ago

Rebasing onto master should sort the JS tests following #119

dgarcia360 commented 3 years ago

@foster999 I added the test πŸ‘ I had to refactor the fixture check_asset_links, please let me know if you like the solution proposed.

foster999 commented 3 years ago

Sorry for the delay! Failing tests here are caused by one of our unpinned dependencies, so I'll get this merged

welcome[bot] commented 3 years ago

Congrats on your first merged pull request in this project! :tada: congrats
Thank you for contributing, we are very proud of you! :heart:

0dminnimda commented 3 years ago

At the moment, you can use main, but for the future, in which version will this functionality be added?

foster999 commented 3 years ago

Should be available on 3.1.0 now πŸ˜„