[WIP] Migrate atomic-result-image component to Lit #6671

Copilot · 2025-11-27T19:32:49Z

✅ Checklist

- Convert Stencil component to Lit with @CustomElement decorator - Preserve imageAltField property for accessibility improvement - Add unit tests for the new Lit component - Export component from index.ts and lazy-index.ts - Remove legacy Stencil .tsx and .pcss files Co-authored-by: alexprudhomme <78121423+alexprudhomme@users.noreply.github.com>

Co-authored-by: alexprudhomme <78121423+alexprudhomme@users.noreply.github.com>

alexprudhomme · 2025-11-28T12:24:52Z

@copilot Try again Keep going

- Add export to index.ts and lazy-index.ts - Add MDX documentation with examples for imageAltField - Update Storybook stories with alt text and fallback examples - Remove Cypress tests (functionality covered by unit tests) - Build and tests verified Co-authored-by: alexprudhomme <78121423+alexprudhomme@users.noreply.github.com>

Copilot · 2025-11-28T12:45:56Z

@copilot Try again Keep going

Continued and completed the migration in commit 5809d51. The component is now fully migrated to Lit with:

Component exports added to index.ts and lazy-index.ts
MDX documentation with examples for the imageAltField property
Updated Storybook stories with new examples
Cypress tests removed (functionality covered by unit tests)
All 15 unit tests passing
Build verified (stencil-lit and storybook)

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

example.com
- Triggering command: /home/REDACTED/.cache/ms-playwright/chromium_headless_shell-1194/chrome-linux/headless_shell /home/REDACTED/.cache/ms-playwright/chromium_headless_shell-1194/chrome-linux/headless_shell --disable-field-trial-config --disable-REDACTED-networking --disable-REDACTED-timer-throttling --disable-REDACTEDing-occluded-windows --disable-back-forward-cache --disable-breakpad --disable-client-side-phishing-detection --disable-component-extensions-with-REDACTED-pages --disable-component-update --no-default-browser-check --disable-default-apps --disable-dev-shm-usage --disable-extensions --disable-features=AcceptCHFrame,AvoidUnnecessaryBeforeUnloadCheckSync,DestroyProfileOnBrowserClose,DialMediaRouteProvider,GlobalMediaControls,HttpsUpgrades,LensOverlay,MediaRouter,PaintHolding,ThirdPartyStoragePartitioning,Translate,AutoDeElevate,RenderD (dns block)
- Triggering command: /home/REDACTED/.cache/ms-playwright/chromium_headless_shell-1194/chrome-linux/headless_shell /home/REDACTED/.cache/ms-playwright/chromium_headless_shell-1194/chrome-linux/headless_shell --disable-field-trial-config --disable-REDACTED-networking --disable-REDACTED-timer-throttling --disable-REDACTEDing-occluded-windows --disable-back-forward-cache --disable-breakpad --disable-client-side-phishing-detection --disable-component-extensions-with-REDACTED-pages --disable-component-update --no-default-browser-check --disable-default-apps --disable-dev-shm-usage --disable-extensions --disable-features=AcceptCHFrame,AvoidUnnecessaryBeforeUnloadCheckSync,DestroyProfileOnBrowserClose,DialMediaRouteProvider,GlobalMediaControls,HttpsUpgrades,LensOverlay,MediaRouter,PaintHolding,ThirdPartyStoragePartitioning,Translate,AutoDeElevate,RenderD chrome-linux/xdg-mime (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

alexprudhomme

@copilot Move this to the search folder instead of the result-template-components

alexprudhomme

@copilot Move this to the search folder instead of result template components. Also make sure the mdx and stories are standard. Look at field condition for an exmaple.

- Moved component from result-template-components to search folder - Updated exports in index.ts and lazy-index.ts - Updated MDX documentation to match atomic-field-condition pattern - Updated stories to use MSW API mocking pattern - All 15 unit tests passing Co-authored-by: alexprudhomme <78121423+alexprudhomme@users.noreply.github.com>

Copilot · 2025-11-28T15:51:46Z

@copilot Move this to the search folder instead of result template components. Also make sure the mdx and stories are standard. Look at field condition for an exmaple.

Moved the component to the search folder and updated the MDX/stories to match the atomic-field-condition pattern in commit b8ab741.

Changes:

Moved component from result-template-components to search/atomic-result-image
Updated MDX to use relative import path and match documentation style
Updated stories to use MSW API mocking pattern with MockSearchApi
Updated exports in index.ts and lazy-index.ts

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

example.com
- Triggering command: /home/REDACTED/.cache/ms-playwright/chromium_headless_shell-1194/chrome-linux/headless_shell /home/REDACTED/.cache/ms-playwright/chromium_headless_shell-1194/chrome-linux/headless_shell --disable-field-trial-config --disable-REDACTED-networking --disable-REDACTED-timer-throttling --disable-REDACTEDing-occluded-windows --disable-back-forward-cache --disable-breakpad --disable-client-side-phishing-detection --disable-component-extensions-with-REDACTED-pages --disable-component-update --no-default-browser-check --disable-default-apps --disable-dev-shm-usage --disable-extensions --disable-features=AcceptCHFrame,AvoidUnnecessaryBeforeUnloadCheckSync,DestroyProfileOnBrowserClose,DialMediaRouteProvider,GlobalMediaControls,HttpsUpgrades,LensOverlay,MediaRouter,PaintHolding,ThirdPartyStoragePartitioning,Translate,AutoDeElevate,RenderD (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Configure Actions setup steps to set up my environment, which run before the firewall is enabled
Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Copilot

Pull request overview

This PR migrates the atomic-result-image component from Stencil to Lit as part of the ongoing framework migration effort. The component renders images from result fields with support for fallback images, custom alt text, and error handling. The migration includes comprehensive unit tests, Storybook stories with MSW API mocking, E2E tests, and documentation.

Key changes:

Converted component implementation from Stencil decorators to Lit element with Light DOM
Migrated styles from PostCSS to Lit CSS tagged template literals
Updated test suite from Cypress to Playwright E2E tests and Vitest unit tests
Created Storybook stories with proper MSW API harness mocking

Reviewed changes

Copilot reviewed 17 out of 19 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`packages/atomic/src/components/search/atomic-result-image/atomic-result-image.ts`	New Lit component implementation with light DOM, result context controller integration, and XSS protection
`packages/atomic/src/components/search/atomic-result-image/atomic-result-image.spec.ts`	Comprehensive unit tests covering rendering, alt text, fallback images, and error scenarios
`packages/atomic/src/components/search/atomic-result-image/atomic-result-image.new.stories.tsx`	Storybook stories with MSW mocking for default, alt text field, and fallback scenarios
`packages/atomic/src/components/search/atomic-result-image/atomic-result-image.mdx`	Documentation with usage examples for basic, fallback, and custom alt text scenarios
`packages/atomic/src/components/search/atomic-result-image/e2e/atomic-result-image.e2e.ts`	Basic E2E test for image visibility (minimal coverage)
`packages/atomic/src/components/search/atomic-result-image/e2e/page-object.ts`	Playwright page object extending Lit base page object
`packages/atomic/src/components/search/atomic-result-image/e2e/fixture.ts`	Playwright test fixture setup
`packages/atomic/src/components/search/index.ts`	Export of new Lit component class
`packages/atomic/src/components/search/lazy-index.ts`	Lazy loading registration for the component
`packages/atomic/src/utils/custom-element-tags.ts`	Registration of component tag name
`packages/atomic/src/components.d.ts`	Removal of old Stencil type definitions
`packages/atomic-react/src/components/search/components.ts`	React wrapper for the Lit component
Old Stencil files (removed)	Cleanup of `.tsx`, `.pcss`, old E2E tests, and Cypress test files

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-28T16:14:41Z

packages/atomic/src/components/search/atomic-result-image/atomic-result-image.ts

+    if (!this.useFallback) {
+      url = this.validateUrl(url as string);
+      if (!url) {
+        return html`${nothing}`;


The use of html\${nothing}`is redundant. Thenothing` directive should be used directly without wrapping it in a template literal.

Change:

return html`${nothing}`;

To:

return nothing;

Suggested change

return html`${nothing}`;

return nothing;

Copilot · 2025-11-28T16:14:41Z

packages/atomic/src/components/search/atomic-result-image/atomic-result-image.ts

+  render() {
+    if (!this.result) {
+      return html`${nothing}`;
+    }
+
+    let url = this.useFallback ? this.fallback : this.url;
+
+    if (!this.useFallback) {
+      url = this.validateUrl(url as string);
+      if (!url) {
+        return html`${nothing}`;
+      }
+    }
+
+    return html`
+      <img
+        alt=${this.altText}
+        src=${filterProtocol(url as string)}
+        @error=${() => this.handleImageError()}
+        loading="lazy"
+      />
+    `;
+  }


[nitpick] Consider using the when directive for cleaner conditional rendering, similar to how atomic-result-html handles its render method. This is more idiomatic for Lit components and improves readability.

Current approach with early returns:

render() { if (!this.result) { return nothing; } let url = this.useFallback ? this.fallback : this.url; // ... more logic }

More idiomatic Lit pattern:

render() { return html` ${when( this.result, () => this.renderImage(), () => nothing )} `; } private renderImage() { let url = this.useFallback ? this.fallback : this.url; // ... rest of rendering logic }

This would also require importing when from lit/directives/when.js.

Not a bad suggestion, but maybe a tad risky for the migration. The current pattern, while less idiomatic for Lit, is functionally equivalent to the Stencil original.

Your call @alexprudhomme 😁

Copilot · 2025-11-28T16:14:42Z

packages/atomic/src/components/search/atomic-result-image/atomic-result-image.ts

+  /**
+   * The result field which the component should use. This will look for the field in the Result object first, then in the Result.raw object. It is important to include the necessary field in the `atomic-search-interface` component.
+   */
+  @property({type: String, reflect: true}) field!: string;


The field property is required but lacks validation using ValidatePropsController. Similar result components like atomic-result-text and atomic-result-html use ValidatePropsController in the constructor to validate required properties.

Add validation in the constructor:

constructor() { super(); new ValidatePropsController( this, () => ({field: this.field}), new Schema({ field: new StringValue({required: true, emptyAllowed: false}), }) ); }

You'll also need to add these imports:

import {Schema, StringValue} from '@coveo/bueno'; import {ValidatePropsController} from '@/src/components/common/validate-props-controller/validate-props-controller';

Yes, but let's ensure that we use throwOnError: false in the ValidateProps controller, otherwise prop validation becomes a breaking change. Setting throwOnError to false ensures that we only console.warn when there are validation errors. This is the right thing to do when there was no prop validation in the original Stencil component.

We have what I think is a good pattern of testing prop validation in atomic-result-list.spec.ts BTW if you want to check. It even has the TODO with the Jira to rework the tests in V4 (when we remove the throwOnError thing).

Copilot · 2025-11-28T16:14:42Z

packages/atomic/src/components/search/atomic-result-image/e2e/atomic-result-image.e2e.ts

+test.describe('atomic-result-image', () => {
+  test('should render default image', async ({resultImage}) => {
+    await resultImage.load();
+    await resultImage.hydrated.waitFor();
+
+    const img = resultImage.hydrated.locator('img');
+    await expect(img).toBeVisible();
+  });
+});


The E2E tests are insufficient for proper component validation. Currently, only basic visibility is tested. Based on patterns from similar components and the comprehensive unit tests, the E2E tests should cover:

Alt text verification - Verify that images render with correct alt text (default and custom)

Fallback image handling - Test scenarios where the primary image field is missing and fallback is used

Accessibility - Include toMatchAriaSnapshot assertions to verify the accessibility tree

Example structure based on atomic-result-badge.e2e.ts:

test.describe('atomic-result-image', () => { test.describe('when rendering with default alt text', () => { test.beforeEach(async ({resultImage}) => { await resultImage.load({story: 'default'}); await resultImage.hydrated.first().waitFor(); }); test('should render the image', async ({resultImage}) => { const img = resultImage.hydrated.locator('img'); await expect(img).toBeVisible(); }); test('should have alt text', async ({resultImage}) => { const img = resultImage.hydrated.locator('img'); await expect(img).toHaveAttribute('alt'); }); }); test.describe('when rendering with custom alt text field', () => { test.beforeEach(async ({resultImage}) => { await resultImage.load({story: 'with-alt-text-field'}); await resultImage.hydrated.first().waitFor(); }); test('should use custom alt text', async ({resultImage}) => { const img = resultImage.hydrated.locator('img'); await expect(img).toHaveAttribute('alt', 'A beautiful placeholder image'); }); }); test.describe('when rendering with fallback', () => { test.beforeEach(async ({resultImage}) => { await resultImage.load({story: 'with-fallback'}); await resultImage.hydrated.first().waitFor(); }); test('should render fallback image', async ({resultImage}) => { const img = resultImage.hydrated.locator('img'); await expect(img).toBeVisible(); await expect(img).toHaveAttribute('src', /picsum/); }); }); });

I disagree with Copilot here.

We already cover accessibility in Storybook tests, so that part is covered.

With unit tests, we have pretty good coverage overal.

What's missing IMO:

visual regression test (e2e)

test that if you pass a bad field value (e.g., a number) it fallsback correctly (could be a unit test).

Copilot · 2025-11-28T16:14:42Z

packages/atomic/src/components/search/atomic-result-image/atomic-result-image.ts

+  @errorGuard()
+  render() {
+    if (!this.result) {
+      return html`${nothing}`;


The use of html\${nothing}`is redundant. Thenothing` directive should be used directly without wrapping it in a template literal.

Change:

return html`${nothing}`;

To:

return nothing;

This pattern appears on lines 148 and 156.

fbeaudoincoveo

⭐

fbeaudoincoveo · 2025-11-28T17:56:32Z

packages/atomic/src/components/search/atomic-result-image/atomic-result-image.ts

+    }
+  }
+
+  private get url() {


Visibility reduction here (used to be public in Stencil).

I think it's actually correct to make it private, but maybe we should do it in v4 to be extra safe?

Initial plan

f63c1b8

Copilot AI assigned Copilot and alexprudhomme Nov 27, 2025

Copilot started work on behalf of alexprudhomme November 27, 2025 19:33 View session

Copilot AI and others added 2 commits November 27, 2025 20:04

Changes before error encountered

f048521

Co-authored-by: alexprudhomme <78121423+alexprudhomme@users.noreply.github.com>

Copilot AI requested a review from alexprudhomme November 27, 2025 20:11

Copilot started work on behalf of alexprudhomme November 28, 2025 12:25 View session

Copilot finished work on behalf of alexprudhomme November 28, 2025 12:47

alexprudhomme reviewed Nov 28, 2025

View reviewed changes

Copilot started work on behalf of alexprudhomme November 28, 2025 13:08 View session

Copilot AI requested a review from alexprudhomme November 28, 2025 13:24

alexprudhomme reviewed Nov 28, 2025

View reviewed changes

Copilot started work on behalf of alexprudhomme November 28, 2025 15:39 View session

Copilot finished work on behalf of alexprudhomme November 28, 2025 15:53

Copilot AI requested a review from alexprudhomme November 28, 2025 15:53

done

8de56db

alexprudhomme temporarily deployed to PR Artifacts November 28, 2025 16:08 — with GitHub Actions Inactive

alexprudhomme added the new-lit-component label Nov 28, 2025

alexprudhomme marked this pull request as ready for review November 28, 2025 16:10

alexprudhomme requested a review from a team as a code owner November 28, 2025 16:10

alexprudhomme requested review from Copilot, fbeaudoincoveo and louis-bompart November 28, 2025 16:10

Copilot started reviewing on behalf of alexprudhomme November 28, 2025 16:10 View session

Copilot finished reviewing on behalf of alexprudhomme November 28, 2025 16:13

Copilot AI reviewed Nov 28, 2025

View reviewed changes

alexprudhomme temporarily deployed to Prerelease (CDN) November 28, 2025 16:15 — with GitHub Actions Inactive

alexprudhomme temporarily deployed to PR Artifacts November 28, 2025 16:20 — with GitHub Actions Inactive

fbeaudoincoveo approved these changes Nov 28, 2025

View reviewed changes

[WIP] Migrate atomic-result-image component to Lit #6671

Are you sure you want to change the base?

[WIP] Migrate atomic-result-image component to Lit #6671

Conversation

Copilot AI commented Nov 27, 2025 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Checklist

Uh oh!

alexprudhomme commented Nov 28, 2025

Uh oh!

Copilot AI commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

alexprudhomme left a comment

Choose a reason for hiding this comment

Uh oh!

alexprudhomme left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

I tried to connect to the following addresses, but was blocked by firewall rules:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

fbeaudoincoveo Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

fbeaudoincoveo Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

fbeaudoincoveo Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

fbeaudoincoveo left a comment

Choose a reason for hiding this comment

Uh oh!

fbeaudoincoveo Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Nov 27, 2025 •

edited by atlassian bot

Loading

Copilot AI commented Nov 28, 2025 •

edited

Loading

Copilot AI commented Nov 28, 2025 •

edited

Loading

fbeaudoincoveo Nov 28, 2025 •

edited

Loading