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.

Sign up for GitHub

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

Jump to bottom

Support inline SVG images #2634

Merged
merged 2 commits into from Oct 17, 2019
Merged

Support inline SVG images #2634

merged 2 commits into from Oct 17, 2019

Conversation

stuaxo
Copy link
Contributor

@stuaxo stuaxo commented Oct 10, 2019

PR for #2561

@justinmayer
Copy link
Member

Many thanks for the contribution, Stuart!

@getpelican/reviewers / @bberberov: Any feedback on this implementation?

@stuaxo
Copy link
Contributor Author

stuaxo commented Oct 15, 2019

I realised I haven't tried the HTML file in a browser separately yet, it's worth verifying that browsers are happy with it.

(I remember trying inline SVG years ago and having trouble if I included the namespace).

iKevinY
iKevinY approved these changes Oct 15, 2019
View reviewed changes
Copy link
Member

@iKevinY iKevinY left a comment

Choose a reason for hiding this comment

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

Don't know much about the specifics of inline SVGs, but this change LGTM!

avaris
avaris approved these changes Oct 15, 2019
View reviewed changes
Copy link
Member

@avaris avaris left a comment

Choose a reason for hiding this comment

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

👍

@stuaxo
Copy link
Contributor Author

stuaxo commented Oct 15, 2019

OK, opens in browsers for me, so all good here.

@justinmayer
Copy link
Member

justinmayer commented Oct 15, 2019 •
edited

@stuaxo: Fantastic. Would you be so kind as to add a RELEASE.md file with a patch release type (see the Contributing section of the docs), and then squash any commits that you think don't need to stand on their own? (if any)

@stuaxo stuaxo force-pushed the feature/svg-inline branch 3 times, most recently from 34851db to 8698ba1 Compare October 15, 2019 21:41
@stuaxo
Copy link
Contributor Author

stuaxo commented Oct 17, 2019

@justinmayer should be good to go.

@justinmayer justinmayer changed the title Feature/svg inline Support inline SVG images Oct 17, 2019
@justinmayer justinmayer merged commit 67781f6 into getpelican:master Oct 17, 2019
@justinmayer
Copy link
Member

Many thanks to @stuaxo for the enhancement and to @avaris and @iKevinY for reviewing.

This improvement is included in the just-released Pelican 4.2. ✨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@avaris avaris avaris approved these changes

@iKevinY iKevinY iKevinY approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@stuaxo @justinmayer @avaris @iKevinY