Add height option to stlite directive#38
Conversation
- Add :height: option to customize iframe height - Support units (px, rem) and numeric values (auto-convert to px) - Update documentation with usage examples - Add test cases for height option parsing - Add custom-height.rst example page
There was a problem hiding this comment.
Pull request overview
This PR adds a :height: option to the stlite directive, allowing users to customize the iframe height using inline styles. This provides a cleaner alternative to the existing workaround of using raw HTML and CSS (as seen in plot-histogram-chart.rst).
Changes:
- Added
:height:directive option that accepts CSS units (px, rem) or numeric values (auto-converted to px) - Implemented inline style generation for iframe elements when height is specified
- Added comprehensive documentation and example page
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/atsphinx/stlite/directives.py |
Adds :height: option to directive's option_spec and default options |
src/atsphinx/stlite/nodes.py |
Implements inline style attribute generation for custom heights |
tests/test_directives.py |
Adds parametrized tests for height option parsing (px, rem, numeric) |
docs/guide.rst |
Documents the new :height: directive option with examples |
docs/examples/custom-height.rst |
Provides practical examples demonstrating height customization |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
attakei
left a comment
attakei
left a comment
There was a problem hiding this comment.
@tkoyama010 Thank you for good suggestion! I welcome this features.
By the way, is width option unneed for you? I think that it should be defined both height and width likely image directive.
| option_spec = { | ||
| "config": parsed_dict, | ||
| "requirements": list_of_str, | ||
| "height": str, |
There was a problem hiding this comment.
When define spec of option in option_spec, it should set conversion function instead of type functions.
In this case, it should be used docutils.parsers.rst.directives:unchanged because it uses raw string.
Ref: https://www.docutils.org/docs/howto/rst-directives.html#option-conversion-functions
| if node.attributes.get("height"): | ||
| height = node.attributes["height"] | ||
| # Ensure px suffix if numeric value without unit | ||
| if height.isdigit(): |
There was a problem hiding this comment.
Stylesheet supports float number.
It should use isumeric instead of isdigit.
Summary
Add
:height:option to customize iframe height.Usage
Supports CSS units (
px,rem) and numeric values (auto-converted topx).Changes
:height:option to directiveBackward compatible - default behavior unchanged.
Reference
tkoyama010/awesome-sphinx-revealjs#4