Skip to content

Conversation

@ramonjd
Copy link
Member

@ramonjd ramonjd commented Feb 3, 2022

Description

Following on from the conversation in #35975 (review)

This PR explicitly checks for a return value of false from strpos() just in case the check returns 0, which would also be falsey and satisfy the if-statement's condition.

If data-id were at the leading position in the string though, we'd have other problems :)

Props to @mcsf for catching it.

Testing Instructions

Insert a gallery and a single image into a post. Publish the page and head over to your site.

Make sure that the published gallery images include the data-id attribute on the <img /> tags. The single image will not have the data-id attribute.

Screenshots

Screen Shot 2022-02-04 at 9 40 35 am

Types of changes

Code qual.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).
  • I've updated related schemas if appropriate.

…content, which would satisfy the if-statement's condition.
@ramonjd ramonjd added [Type] Code Quality Issues or PRs that relate to code quality [Block] Image Affects the Image Block labels Feb 3, 2022
@ramonjd ramonjd requested review from glendaviesnz and mcsf February 3, 2022 22:53
@ramonjd ramonjd requested a review from ajitbohra as a code owner February 3, 2022 22:53
@ramonjd ramonjd self-assigned this Feb 3, 2022
Copy link
Contributor

@aaronrobertshaw aaronrobertshaw left a comment

Choose a reason for hiding this comment

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

LGTM 👍

✅ Gallery images get the data-id attribute
✅ Single images do not get the data-id attribute

@ramonjd ramonjd merged commit 037c82b into trunk Feb 4, 2022
@ramonjd ramonjd deleted the update/image-block-attribute-strpos-check branch February 4, 2022 02:26
@github-actions github-actions bot added this to the Gutenberg 12.6 milestone Feb 4, 2022
@mcsf
Copy link
Contributor

mcsf commented Feb 4, 2022

Thanks!

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

Labels

[Block] Image Affects the Image Block [Type] Code Quality Issues or PRs that relate to code quality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants