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

Support for relative width #26

Merged
merged 3 commits into from
Nov 11, 2024

Conversation

AnonymouX47
Copy link
Contributor

  • The width option can now accept an integer percentage as its value.
  • When width is relative, height is ignored and a warning is logged.
  • Updates the descriptions of height and width options and uses relative width in the existing example.
  • Corrects the options used in the example code block to match what is actually used in the video directive.

docs/quickstart.rst Show resolved Hide resolved
docs/quickstart.rst Show resolved Hide resolved
Copy link
Member

@12rambau 12rambau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you think if we go one step further and accept any valid css descriptor ? (em, rem, %, vw, vh, pixel)
If you don't have time it can be done in a follow-up PR.

@AnonymouX47
Copy link
Contributor Author

AnonymouX47 commented Apr 21, 2023

Actually, I have little to no knowledge of HTML and CSS.

I just searched up how to do this specifically and mokey-patched the Video class from within my conf.py. Then, I thought it could actually be useful as a feature and decided to contribute it.

@Borda
Copy link
Contributor

Borda commented Jun 28, 2023

This would be great to land together with max-width 🐰

Copy link
Member

@12rambau 12rambau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AnonymouX47 sorry for the long wait, all my OSS time was eaten up by another project. I would like to move forward with your PR, could you rebase it with the current main so I can merge it in the next release ?

- Change: The `width` option can now accept an integer percentage as
  its value.
- Change: When `width` is relative, `height` is ignored and a warning
  is logged.
- Fix: Correct the option usage example.
- Change: Update the descriptions of `height` and `width` options.
- Change: Use relative width in the option usage example.
@AnonymouX47
Copy link
Contributor Author

All done, I believe.

@12rambau 12rambau merged commit a817d1c into sphinx-contrib:master Nov 11, 2024
6 checks passed
@AnonymouX47 AnonymouX47 deleted the relative-width branch November 11, 2024 10:13
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