Instant Search: add image alt text from API #22295

bluefuton · 2022-01-11T03:26:40Z

Fixes #19273.

Changes proposed in this Pull Request:

The Instant Search API now provides image.alt_text as a field we can request.

This PR adds the new field and switches to use it for alt text in the expanded and product result formats.

I've also reduced the fields fetched for the minimal result format (image fields are not needed).

Does this pull request change what data or activity we track or use?

No.

Testing instructions

On a site with Jetpack Instant Search enabled, create a post with an image.
Set alt text for the image and ensure that the alt text appears on images in search results too (/?s=).

github-actions · 2022-01-11T03:28:07Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
⚠️ All commits were linted before commit.
✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

Jetpack plugin:

Next scheduled release: February 1, 2022.
Scheduled code freeze: January 24, 2022.

bluefuton · 2022-01-12T02:51:15Z

projects/packages/search/src/instant-search/components/search-result-expanded.jsx

@@ -77,7 +77,7 @@ export default function SearchResultExpanded( props ) {
 					<div className="jetpack-instant-search__search-result-expanded__image-container">
 						{ firstImage ? (
 							<PhotonImage
-								alt={ fields[ 'title.default' ] }
+								alt={ fields[ 'image.alt_text' ] }


I thought about keeping title.default as a fallback here, but it's complicated by the fact that alt="" is valid alt text.

Feels best to rely on the alt text from the API in all cases.

kangzj

This tests well and looks good to me 👍
Some checks are failing, and the errors don't seem relevant to the PR - probably just need to rebase and have the checks re-run and see.

bluefuton · 2022-01-13T01:55:23Z

The only required check failing is 'Linting / Changelogger validity (pull_request)':

@Automattic/jetpack-crew can you advise whether we need to use fixup-project-versions here?

anomiex · 2022-01-13T16:37:31Z

@Automattic/jetpack-crew can you advise whether we need to use fixup-project-versions here?

Well, you don't need to, you could do the version bumps manually. 😀 But fixup-project-versions saves the trouble of figuring out all the details.

P.S. if you're on a Mac and that command is really slow, you might see p7H4VZ-3gs-p2 (or https://apple.stackexchange.com/questions/338415/php-incredibly-slow-since-upgrading-macos-to-10-14-mojave)

bluefuton · 2022-01-13T22:38:08Z

Well, you don't need to, you could do the version bumps manually. 😀 But fixup-project-versions saves the trouble of figuring out all the details.

Thanks @anomiex! Do we need to do this on every search PR in the future?

anomiex · 2022-01-14T14:31:52Z

Thanks @anomiex! Do we need to do this on every search PR in the future?

I usually run it on my PRs that touch any project, just before pushing. It doesn't take that long and it'll safely do nothing if nothing is needed.

In general for a composer package it's only needed if you're doing the first "minor" or "major" PR since the last time the package was released (which will generally be weekly, since packages used by Jetpack get released as part of the weekly Jetpack release for Atomic), unless the package is configured with a version number constant.

kangzj · 2022-01-17T01:48:35Z

projects/plugins/jetpack/changelog/add-instant-search-alt-text-where-available#2

@@ -0,0 +1,5 @@
+Significance: patch


Do you think keeping projects/plugins/jetpack/changelog/add-instant-search-alt-text-where-available is sufficient enough?

kangzj

Good job! It looks good and works well 👍

jeherve

I'm afraid this is going to need a rebase now. :(

…instant-search-alt-text-where-available

bluefuton added [Status] In Progress [Feature] Search For all things related to Search Instant Search labels Jan 11, 2022

bluefuton self-assigned this Jan 11, 2022

bluefuton requested a review from a team as a code owner January 11, 2022 03:26

matticbot added Touches WP.com Files labels Jan 11, 2022

github-actions bot added the [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ label Jan 11, 2022

bluefuton mentioned this pull request Jan 11, 2022

Instant Search: use correct image alt text #19273

Closed

bluefuton force-pushed the add/instant-search-alt-text-where-available branch from 677c0be to cb9be12 Compare January 11, 2022 20:55

matticbot removed the Touches WP.com Files label Jan 11, 2022

github-actions bot added the [Package] Search Contains core Search functionality for Jetpack and Search plugins label Jan 11, 2022

bluefuton commented Jan 12, 2022

View reviewed changes

bluefuton added [Status] Needs Team Review and removed [Status] In Progress labels Jan 12, 2022

kangzj reviewed Jan 12, 2022

View reviewed changes

bluefuton force-pushed the add/instant-search-alt-text-where-available branch from ea47400 to 26f082b Compare January 13, 2022 01:23

bluefuton force-pushed the add/instant-search-alt-text-where-available branch from a462b58 to 177ec23 Compare January 13, 2022 22:39

bluefuton requested a review from kangzj January 16, 2022 22:32

kangzj reviewed Jan 17, 2022

View reviewed changes

kangzj previously approved these changes Jan 17, 2022

View reviewed changes

jeherve requested changes Jan 24, 2022

View reviewed changes

jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Team Review labels Jan 24, 2022

[not verified] Instant Search: add alt text from API

9b4d173

bluefuton added 3 commits January 26, 2022 15:08

[not verified] Changelog

d63ce4b

[not verified] Move changelog to package

d5ee13f

Fixup project versions

9118cc8

bluefuton dismissed kangzj’s stale review via 9118cc8 January 26, 2022 02:12

bluefuton force-pushed the add/instant-search-alt-text-where-available branch from 177ec23 to 9118cc8 Compare January 26, 2022 02:12

bluefuton added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Jan 26, 2022

[not verified] Merge remote-tracking branch 'origin/master' into add/…

57d64ba

…instant-search-alt-text-where-available

jeherve approved these changes Jan 28, 2022

View reviewed changes

jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Jan 28, 2022

jeherve added this to the jetpack/10.7 milestone Jan 28, 2022

jeherve merged commit 5d06380 into master Jan 28, 2022

jeherve deleted the add/instant-search-alt-text-where-available branch January 28, 2022 17:01

github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Jan 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Instant Search: add image alt text from API #22295

Instant Search: add image alt text from API #22295

bluefuton commented Jan 11, 2022 •

edited

Loading

github-actions bot commented Jan 11, 2022 •

edited

Loading

bluefuton Jan 12, 2022

kangzj left a comment

bluefuton commented Jan 13, 2022

anomiex commented Jan 13, 2022

bluefuton commented Jan 13, 2022

anomiex commented Jan 14, 2022

kangzj Jan 17, 2022

kangzj left a comment

jeherve left a comment

Instant Search: add image alt text from API #22295

Instant Search: add image alt text from API #22295

Conversation

bluefuton commented Jan 11, 2022 • edited Loading

Changes proposed in this Pull Request:

Does this pull request change what data or activity we track or use?

Testing instructions

github-actions bot commented Jan 11, 2022 • edited Loading

bluefuton Jan 12, 2022

Choose a reason for hiding this comment

kangzj left a comment

Choose a reason for hiding this comment

bluefuton commented Jan 13, 2022

anomiex commented Jan 13, 2022

bluefuton commented Jan 13, 2022

anomiex commented Jan 14, 2022

kangzj Jan 17, 2022

Choose a reason for hiding this comment

kangzj left a comment

Choose a reason for hiding this comment

jeherve left a comment

Choose a reason for hiding this comment

bluefuton commented Jan 11, 2022 •

edited

Loading

github-actions bot commented Jan 11, 2022 •

edited

Loading