-
Notifications
You must be signed in to change notification settings - Fork 78
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
Tools: Resize images in markdown #64
base: main
Are you sure you want to change the base?
Tools: Resize images in markdown #64
Conversation
issue As we are adding a fragment to the URL, I think it should follow the usual URL convention of |
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.
Thank you for the fragment format update 🙌 .
@@ -45,6 +45,5 @@ We are excited to have you here! Our intention for making the project open-sour | |||
| <a href="https://thousandbrains.discourse.group/" style="display: flex; align-items: center;"><img src="../figures/overview/discourse.png" alt="Forum" height="15" style="opacity: 1; transition: opacity 0.2s; &:hover { opacity: 0.8; }" pointer-events="none"> Community Forum</a> | Discuss ideas, ask questions and connect with other community members | | |||
| <a href="https://thousandbrains.org/" style="display: flex; align-items: center;"><img src="../figures/overview/website.png" alt="Website" height="15" style="opacity: 1; transition: opacity 0.2s; &:hover { opacity: 0.8; }" pointer-events="none"> Official Website</a> | Learn about our mission, team and the science behind the project | | |||
| <a href="https://www.youtube.com/@thousandbrainsproject" style="display: flex; align-items: center;"><img src="../figures/overview/youtube.png" alt="YouTube" height="15" style="opacity: 1; transition: opacity 0.2s; &:hover { opacity: 0.8; }" pointer-events="none"> YouTube Channel</a> | Watch tutorials, technical deep-dives and project updates | | |||
| <a href="https://bsky.app/profile/1000brainsproj.bsky.social" style="display: flex; align-items: center;"><img src="../figures/overview/bluesky.png" alt="Bluesky" height="15" style="opacity: 1; transition: opacity 0.2s; &:hover { opacity: 0.8; }" pointer-events="none"> Bluesky</a>| Get the latest news and announcements, and engage with our community | | |||
|<a href="https://x.com/1000brainsproj" style="display: flex; align-items: center;"><img src="../figures/overview/twitter.png" alt="Twitter" height="15" style="opacity: 1; transition: opacity 0.2s; &:hover { opacity: 0.8; }" pointer-events="none"> Twitter/X</a> | Get the latest news and announcements, and engage with our community | | |||
| <a href="https://bsky.app/profile/1000brainsproj.bsky.social" style="display: flex; align-items: center;"><img src="../figures/overview/bluesky.png" alt="Bluesky" height="15" style="opacity: 1; transition: opacity 0.2s; &:hover { opacity: 0.8; }" pointer-events="none"> Bluesky</a> <a href="https://x.com/1000brainsproj" style="display: flex; align-items: center;"><img src="../figures/overview/twitter.png" alt="Twitter" height="15" style="opacity: 1; transition: opacity 0.2s; &:hover { opacity: 0.8; }" pointer-events="none"> Twitter</a> | Get the latest news and announcements, and engage with our community | |
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.
suggestion (non-blocking): I think it should be called X here.
clean_src = src_parts[0] | ||
style = "border-radius: 8px;" | ||
|
||
# Parse and sanitize style parameters from fragment |
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.
issue (blocking): I would like to see two things that reinforce each other: a sanitization library and an allowlist of supported keys.
Please look at the nh3
sanitization library to sanitize input: https://nh3.readthedocs.io/en/latest/#. I don't want us to maintain our own sanitization code. I would rather rely on the expertise of people who care about this sort of thing.
After the CSS keys are extracted, they should be checked against an allowlist of keys, in this case, consisting of a single one: width
. However, I noticed height
in the tests. Whatever the list, I think we should enumerate an allowlist to vastly reduce what can be put into the HTML.
One way to accomplish the above would be to first filter key, values
for the allowlist of keys, i.e., width
. Then, second, naively and unsafely construct the final HTML <figure><img...
and then put the final HTML through the nh3.clean()
before returning it.
This PR adds the functionality to add style attributes to images by using the fragment part of a markdown image URL.
For example
Will result in