Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Added alt to avatar #1226

Merged
merged 2 commits into from
Sep 20, 2017
Merged

Added alt to avatar #1226

merged 2 commits into from
Sep 20, 2017

Conversation

lsolesen
Copy link
Contributor

@lsolesen lsolesen commented Sep 5, 2017

No description provided.

@@ -1,6 +1,6 @@
<article id="comment{{ include.index }}" class="js-comment comment" itemprop="comment" itemscope itemtype="http://schema.org/Comment">
<div class="comment__avatar-wrapper">
<img class="comment__avatar" src="https://www.gravatar.com/avatar/{{ include.email }}?d=mm&s=80">
<img class="comment__avatar" src="https://www.gravatar.com/avatar/{{ include.email }}?d=mm&s=80" alt="Avatar for {{ include.name }}">
Copy link
Owner

@mmistakes mmistakes Sep 5, 2017

Choose a reason for hiding this comment

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

Will probably want to use the same method the rest of the theme uses for "localizing" text strings so it can be translated if need be.

Something like alt="{{ site.data.ui-text[site.locale].avatar_for | default: "Avatar for" }} for {{ include.name }}"

And then add avatar_for: "Avatar for" under the English locales in _data/ui-txt.yml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the Avatar part which seems to be more clever just to have the name instead of having to localize. I could do what you originally suggested also, if you think that it is more clever.

@mmistakes mmistakes merged commit d81af1f into mmistakes:master Sep 20, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants