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

Fix problems with some themes #54

Merged

Conversation

s-weigand
Copy link
Contributor

This fixes some issues with some specific themes (see).

Some problems are a result of css rules being overwritten by theme rules (astropy-theme, py3doc-enhanced-theme, typlog-theme), others are a result of an old way (julia-theme, pytorch-theme, itcase-theme, pyviz-theme) to include additional javascript files in a theme.

Details concerning javascript issues

The old way to include javascript files (<script type="text/javascript" src="{{ pathto(scriptfile, 1) }}"></script>) does not support a missing file name so the browser tries to fetch a file None, which leaves copybuttonSkipText as undefined and breaks the copy functionality. If a theme uses js_tag to include javascript files for sphinx>=1.8 it works fine (maybe sphinx>=1.8 should also be a minimal requirement for this package?).

If a theme doesn't have the following part (itcase-theme, pyviz-theme):

<script type="text/javascript">
        var DOCUMENTATION_OPTIONS = {
            URL_ROOT:'{{ url_root }}',
            VERSION:'{{ release|e }}',
            COLLAPSE_INDEX:false,
            FILE_SUFFIX:'{{ '' if no_search_suffix else file_suffix }}',
            HAS_SOURCE:  {{ has_source|lower }},
            SOURCELINK_SUFFIX: '{{ sourcelink_suffix }}'
        };
</script>

copybutton.js does throw the following error copybutton.js:105 Uncaught ReferenceError: DOCUMENTATION_OPTIONS is not defined in the console (problem with clipboardButton) and even the icons aren't generated, due to early breaking.

IMHO this should be mentioned in the docs, maybe in a topic like theme support/requirements, so theme creators have a reference , if they want to support sphinx-copybutton

theme css rules applied on 'img' tags (themes: astropy-theme, py3doc-enhanced-theme, typlog-theme)
caused copy function to break (themes: julia-theme, pytorch-theme)
red line inside the icon with typlog-theme
theme: itcase-theme after js and css inclusion in the template is fixed
warning in the console, explaining what went wrong and how to fix it
@choldgraf
Copy link
Member

choldgraf commented Jan 20, 2020

This all seems reasonable to me - thanks very much for double-checking the behavior of SCB on multiple themes! If you think there's a good place in the docs to mention this, I'd be happy to merge a PR that adds this in somewhere. Let me know if you'd like that to be in this PR or another

As an aside, have you published a circle configuration that demonstrates behavior across a number of themes anywhere? I'd love to implement this in sphinx-copybutton and feel like it'd be helpful for other developers as well!

@mgeier
Copy link
Contributor

mgeier commented Jan 20, 2020

Mentioning the problematic themes in the docs might be a good idea, but I think someone should also create issues (or even PRs) in the repos of the affected themes.
Probably some of the problems can be solved quickly?

@choldgraf choldgraf merged commit 105fa2b into executablebooks:master Jan 23, 2020
@choldgraf
Copy link
Member

it sounds like the docs change is best in a different PR, thanks very much for the improvement @s-weigand !

@s-weigand s-weigand deleted the fix-problems-with-some-themes branch January 24, 2020 14:22
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