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

Add title to blog index page and image to newsletter #252

Merged
merged 3 commits into from
Feb 1, 2024
Merged

Conversation

ericholscher
Copy link
Member

@ericholscher ericholscher commented Feb 1, 2024

@ericholscher ericholscher requested a review from a team as a code owner February 1, 2024 21:03
@ericholscher ericholscher changed the title Add title to blog index page Add title to blog index page and image to newsletter Feb 1, 2024
@ericholscher
Copy link
Member Author

Gonna merge this so it goes out with the newsletter, but post-merge review should be fine here.

@ericholscher ericholscher merged commit 6fd5d4f into main Feb 1, 2024
4 checks passed
@@ -53,7 +53,7 @@

<div class="ui divider"></div>

<img class="ui big rounded centered image" src="{{ article.image or '/images/posts/default.svg' }}" alt="{{ article.image_credit }}">
<img class="ui big rounded centered image" src="{{ article.image or '/images/posts/default.svg' }}" alt="{{ article.image }}">
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the alt was replaced with the image URL. I think this was correct before, no?

Copy link
Contributor

@agjohnson agjohnson Feb 2, 2024

Choose a reason for hiding this comment

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

Yeah, the issue comes from using HTML in the image_credit, which isn't valid in an HTML attribute. I would use striptags or escape the HTML instead of replacing with the URL.

With striptags:

image

Edit: added in #253

agjohnson added a commit that referenced this pull request Feb 2, 2024
PR #252 dropped the alt text to support HTML in the image credits, but a
more accessible fix here is just strip the HTML from the image credits
when using the credits in the attribute.
@stsewd stsewd deleted the blog-title branch February 2, 2024 18:55
ericholscher pushed a commit that referenced this pull request Feb 2, 2024
PR #252 dropped the alt text to support HTML in the image credits, but a
more accessible fix here is just strip the HTML from the image credits
when using the credits in the attribute.
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.

2 participants