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

Try adding alt text when rendering local avatars #127

Merged
merged 3 commits into from
May 17, 2022
Merged

Conversation

dkotter
Copy link
Collaborator

@dkotter dkotter commented May 10, 2022

Description of the Change

As mentioned in #126, it doesn't appear the alt text saved with an attachment is ever used when rendering a local avatar. If someone passes in some custom alt text (for instance, using the 4th argument of get_avatar) that is then used as the alt text for the image. But if that argument isn't passed in but the attachment we are rendering has alt text assigned to it, we should use that for the alt text argument.

This PR fixes that by adding a check that runs if a local avatar is found AND no custom alt text has been passed in. It then pulls the alt text from the underlying attachment object which will be stored in post meta (if it exists, otherwise just an empty string).

I also added a new method used to get the user ID, to avoid having duplicate code between the existing get_simple_local_avatar_url method and the new get_simple_local_avatar_alt method.

Closes #126

Alternate Designs

Right now we default to using the alt text that is passed in and then fallback to the alt text assigned to an image. I think that's the right approach but we could flip those, so the alt text saved with an image would take precedence.

Possible Drawbacks

None

Verification Process

  1. Upload a new image
  2. Add alt text to this image
  3. Set this image as your local avatar
  4. Publish a new post with that user
  5. Using a theme that renders an author section (like Twenty Twenty One) view the front-end of that post
  6. Inspect the author image and note the alt text being used
  7. For a second test, add a call to get_avatar and pass in the 4th argument for the alt text
  8. Inspect the source again and note the alt text now comes from that function and not the image

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Changelog Entry

Added - If an image used for a local avatar has alt text assigned, ensure that alt text is used when rendering the image

Credits

Props @dkotter, @pixelloop

@dkotter dkotter self-assigned this May 10, 2022
@dkotter dkotter requested a review from faisal-alvi May 10, 2022 17:23
@pixelloop
Copy link

I have tested the new code for includes/class-simple-local-avatars.php and can confirm that it fixes the alt text issue.

@jeffpaul jeffpaul added this to the 2.5.0 milestone May 10, 2022
@dkotter
Copy link
Collaborator Author

dkotter commented May 10, 2022

Thanks so much for reporting and testing this @pixelloop!

Copy link
Member

@faisal-alvi faisal-alvi left a comment

Choose a reason for hiding this comment

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

@dkotter LGTM! 🚀 thanks for the work!

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.

Ensure alt text is output when the avatar image is output
4 participants