-
-
Notifications
You must be signed in to change notification settings - Fork 281
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
Added support for embedding media files from <audio src="..."> and <video src="..."> in the self-contained mode #355
Conversation
…med to a data: URL.
…`/`<video src="...">` in the self-contained mode.
…ude a / to close them.
It turns out that media structured |
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!
Co-authored-by: Yihui Xie <xie@yihui.name>
CLA assistant is saying:
I'll keep trying... |
It seems the CLA assistant could be extremely slow: cla-assistant/cla-assistant#872 I just tried to open https://cla-assistant.io/yihui/xaringan?pullRequest=355 and it failed to load for a few times before it finally showed up. |
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.
The code could be simplified a little bit.
Co-authored-by: Yihui Xie <xie@yihui.name>
Co-authored-by: Yihui Xie <xie@yihui.name>
sprintf('<%s .*?src\\s*=\\s*"', '".*?>', c('audio', 'video', 'source')), | ||
'<audio .*?src\\s*=\\s*"', '".*?>', | ||
'<video .*?src\\s*=\\s*"', '".*?>', | ||
'<source .*?src\\s*=\\s*"', '".*?>', |
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.
Is there a reason that you wanted to revert?
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.
Yeah, the changes broke the CI tests (and my own local knit), and I don't understand the surrounding code well enough to debug it in a timely manner. I figured having it working sooner rather than later was better.
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 see. I know what's going on now. I meant something like
sprintf('<%s .*?src\\s*=\\s*"', c('audio', 'video', 'source')), rep('".*?>', 3)
I'll tweak this part after merging this PR. Thanks!
To contribute to this repo, please make sure to:
Thank you!