Skip to content

Commit

Permalink
[EmptyState/Image] Fix image loading with ref (#11874)
Browse files Browse the repository at this point in the history
### WHY are these changes introduced?

Resolves #11856.

EmptyState styles for loaded image were

### WHAT is this pull request doing?

Refactors original logic from
[#11804](#11804) to use an
`HTMLImageElement` ref and the [complete
attribute](https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/complete)
to set loaded image styles.
  <details>
    <summary>EmptyState — before</summary>
<img
src="https://github.com/Shopify/polaris/assets/26749317/959a3bc9-d270-44fa-a916-6655532f82e5"
alt="EmptyState — before">
  </details>
  <details>
    <summary>EmptyState — after</summary>
<img
src="https://github.com/Shopify/polaris/assets/26749317/9304f32d-a95b-41ea-add9-0e37a01a2704"
alt="EmptyState — after">
  </details>

### How to 🎩


[Storybook](https://5d559397bae39100201eedc1-oumakptmqa.chromatic.com/?path=/story/all-components-emptystate--default)

[Spin](https://admin.web.fix-empty-state-2.lo-kim.us.spin.dev/store/shop1/orders)

- For testing Spin instance, it helps to throttle the network to ensure
there's enough time to see the transition between placeholder and final
loaded asset

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#install-dependencies-and-build-workspaces)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

### 🎩 checklist

- [x] Tested a
[snapshot](https://github.com/Shopify/polaris/blob/main/documentation/Releasing.md#-snapshot-releases)
- [x] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [x] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [x] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide

---------

Co-authored-by: Kyle Durand <[email protected]>
  • Loading branch information
laurkim and kyledurand committed Apr 10, 2024
1 parent ce6353b commit 7440367
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 35 deletions.
5 changes: 5 additions & 0 deletions .changeset/gorgeous-tomatoes-push.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/polaris': patch
---

Added support for ref to `Image` to handle image load with `EmptyState`
13 changes: 8 additions & 5 deletions polaris-react/src/components/EmptyState/EmptyState.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useState, useCallback} from 'react';
import React, {useState, useRef, useEffect} from 'react';

import {classNames} from '../../utilities/css';
import type {ComplexAction} from '../../types';
Expand Down Expand Up @@ -47,9 +47,10 @@ export function EmptyState({
footerContent,
}: EmptyStateProps) {
const [imageLoaded, setImageLoaded] = useState<boolean>(false);
const imageRef = useRef<HTMLImageElement>(null);

const handleLoad = useCallback(() => {
setImageLoaded(true);
useEffect(() => {
if (imageRef.current?.complete) setImageLoaded(true);
}, []);

const imageClassNames = classNames(
Expand All @@ -62,22 +63,24 @@ export function EmptyState({
<Image
alt=""
role="presentation"
ref={imageRef}
source={largeImage}
className={imageClassNames}
sourceSet={[
{source: image, descriptor: '568w'},
{source: largeImage, descriptor: '1136w'},
]}
sizes="(max-width: 568px) 60vw"
onLoad={handleLoad}
onLoad={() => setImageLoaded(true)}
/>
) : (
<Image
alt=""
role="presentation"
ref={imageRef}
className={imageClassNames}
source={image}
onLoad={handleLoad}
onLoad={() => setImageLoaded(true)}
/>
);

Expand Down
59 changes: 29 additions & 30 deletions polaris-react/src/components/Image/Image.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, {useCallback} from 'react';
import React, {useCallback, forwardRef} from 'react';

interface SourceSet {
source: string;
Expand All @@ -16,34 +16,33 @@ export interface ImageProps extends React.HTMLProps<HTMLImageElement> {
onError?(): void;
}

export function Image({
alt,
sourceSet,
source,
crossOrigin,
onLoad,
className,
...rest
}: ImageProps) {
const finalSourceSet = sourceSet
? sourceSet
.map(({source: subSource, descriptor}) => `${subSource} ${descriptor}`)
.join(',')
: null;
export const Image = forwardRef<HTMLImageElement, ImageProps>(
({alt, sourceSet, source, crossOrigin, onLoad, className, ...rest}, ref) => {
const finalSourceSet = sourceSet
? sourceSet
.map(
({source: subSource, descriptor}) => `${subSource} ${descriptor}`,
)
.join(',')
: null;

const handleLoad = useCallback(() => {
if (onLoad) onLoad();
}, [onLoad]);
const handleLoad = useCallback(() => {
if (onLoad) onLoad();
}, [onLoad]);

return (
<img
alt={alt}
src={source}
crossOrigin={crossOrigin}
className={className}
onLoad={handleLoad}
{...(finalSourceSet ? {srcSet: finalSourceSet} : {})}
{...rest}
/>
);
}
return (
<img
ref={ref}
alt={alt}
src={source}
crossOrigin={crossOrigin}
className={className}
onLoad={handleLoad}
{...(finalSourceSet ? {srcSet: finalSourceSet} : {})}
{...rest}
/>
);
},
);

Image.displayName = 'Image';

0 comments on commit 7440367

Please sign in to comment.