-
Notifications
You must be signed in to change notification settings - Fork 27
Improve TryExamples customization #129
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
Conversation
- and update how try examples config is specified
.try_examples_button_container { | ||
display: flex; | ||
justify-content: flex-end; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Thanks for your patience on the review, I'm just back from PTO.
@@ -28,4 +28,7 @@ | |||
] | |||
} | |||
|
|||
html_static_path = ["_static"] | |||
html_css_files = ["try_examples.css"] | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
display: flex; | ||
justify-content: flex-end; | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -184,23 +224,38 @@ directory of the build directory for the deployed documentation. The format is a | |||
`TryExamples` buttons will be hidden in url pathnames matching at least one of these | |||
patterns, effectively disabling the interactive documentation. In the provided example: | |||
|
|||
* The pattern `".*latest/.*" disables interactive examples for urls for the documentation | |||
* The pattern `"^/latest/.*"` disables interactive examples for urls for the documentation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch.
"button_horizontal_position": directives.unchanged, | ||
"button_vertical_position": directives.unchanged, | ||
"min_height": directives.unchanged, | ||
"example_class": directives.unchanged, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
f" 'top' or 'bottom', received {button_vertical_position}." | ||
) | ||
height = self.options.pop("height", None) | ||
example_class = self.options.pop("example_class", "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a fix for this PR, but do we want to check here that example_class
is a valid class name ? (no space, no dots, no /\ or angle brackets for example) as we do inject it with fstrings it might break the html/css otherwise
"try_examples_default_runtime_config", | ||
default=None, | ||
rebuild=None, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love when a bunch of lines get deleted.
Ok, let's get that in. I'll open an issue to think about validating the height and example_class |
Thanks @Carreau! |
Closes #122, #127
This PR updates
TryExamplesDirective
to use the standard method of allowing custom css in Sphinx to customize the buttons as suggested by Carreau here. It also adds customization ofmin_height
to the config filetry_examples.json
so that the minimum height can bet changed at runtime. An option has been given to the directive to set the specific height of the iframe container for the embedded notebook and themin_height
option has been removed. The specific height overrides the minimum height set intry_examples.json
. I've also attempted to improve the documentation, and have added anexample_class
option to the directive which can be targeted in custom css as suggested by Carreau.