Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions src/Image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { TransformType } from './hooks/useImageTransform';
import useRegisterImage from './hooks/useRegisterImage';
import useStatus from './hooks/useStatus';
import type { ImageElementProps } from './interface';
import { getFetchPriorityProps } from './util';

export interface ImgInfo {
url: string;
Expand Down Expand Up @@ -99,8 +100,17 @@ const ImageInternal: CompoundedComponent<ImageProps> = props => {
// Image
src: imgSrc,
alt,
crossOrigin,
decoding,
draggable,
fetchPriority,
loading,
placeholder,
referrerPolicy,
fallback,
sizes,
srcSet,
useMap,

// Preview
preview = true,
Expand Down Expand Up @@ -163,14 +173,32 @@ const ImageInternal: CompoundedComponent<ImageProps> = props => {
() => {
const obj: ImageElementProps = {};
COMMON_PROPS.forEach((prop: any) => {
if (props[prop] !== undefined) {
obj[prop] = props[prop];
if (prop === 'fetchPriority') {
Object.assign(obj, getFetchPriorityProps(fetchPriority));
} else if (prop === 'crossOrigin' && crossOrigin !== undefined) {
obj.crossOrigin = crossOrigin;
} else if (prop === 'decoding' && decoding !== undefined) {
obj.decoding = decoding;
} else if (prop === 'draggable' && draggable !== undefined) {
obj.draggable = draggable;
} else if (prop === 'loading' && loading !== undefined) {
obj.loading = loading;
} else if (prop === 'referrerPolicy' && referrerPolicy !== undefined) {
obj.referrerPolicy = referrerPolicy;
} else if (prop === 'sizes' && sizes !== undefined) {
obj.sizes = sizes;
} else if (prop === 'srcSet' && srcSet !== undefined) {
obj.srcSet = srcSet;
} else if (prop === 'useMap' && useMap !== undefined) {
obj.useMap = useMap;
} else if (prop === 'alt' && alt !== undefined) {
obj.alt = alt;
}
});

return obj;
},
Comment on lines 165 to 177
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This useMemo callback can be simplified to improve readability and maintainability. The current implementation uses a long if-else if chain, which is verbose and harder to maintain when adding or removing props.

Consider refactoring this to build the props object more dynamically by separating the generic props from the special case for fetchPriority.

    () => {
      const propsToPass = {
        alt,
        crossOrigin,
        decoding,
        draggable,
        loading,
        referrerPolicy,
        sizes,
        srcSet,
        useMap,
      };

      const obj: Partial<ImageElementProps> = {};
      for (const key in propsToPass) {
        if (Object.prototype.hasOwnProperty.call(propsToPass, key) && propsToPass[key] !== undefined) {
          obj[key] = propsToPass[key];
        }
      }

      Object.assign(obj, getFetchPriorityProps(fetchPriority));

      return obj;
    },

COMMON_PROPS.map(prop => props[prop]),
[alt, crossOrigin, decoding, draggable, fetchPriority, loading, referrerPolicy, sizes, srcSet, useMap],
);

// ========================== Register ==========================
Expand Down
2 changes: 1 addition & 1 deletion src/Preview/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ const Preview: React.FC<PreviewProps> = props => {
zIndex,
} = props;

const imgRef = useRef<HTMLImageElement>();
const imgRef = useRef<HTMLImageElement>(null);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
const groupContext = useContext(PreviewGroupContext);
const showLeftOrRightSwitches = groupContext && count > 1;
const showOperationsProgress = groupContext && count >= 1;
Expand Down
1 change: 1 addition & 0 deletions src/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export const COMMON_PROPS: (keyof Omit<ImageElementProps, 'src'>)[] = [
'crossOrigin',
'decoding',
'draggable',
'fetchPriority',
'loading',
'referrerPolicy',
'sizes',
Expand Down
7 changes: 6 additions & 1 deletion src/interface.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
export type FetchPriority = 'high' | 'low' | 'auto';

/**
* Used for PreviewGroup passed image data
*/
Expand All @@ -7,13 +9,16 @@ export type ImageElementProps = Pick<
| 'crossOrigin'
| 'decoding'
| 'draggable'
| 'fetchPriority'
| 'loading'
| 'referrerPolicy'
| 'sizes'
| 'srcSet'
| 'useMap'
| 'alt'
>;
> & {
fetchpriority?: FetchPriority;
};

export type PreviewImageElementProps = {
data: ImageElementProps;
Expand Down
13 changes: 13 additions & 0 deletions src/util.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import * as React from 'react';
import type { FetchPriority } from './interface';

export function isImageValid(src: string) {
return new Promise(resolve => {
if (!src) {
Expand All @@ -11,6 +14,16 @@ export function isImageValid(src: string) {
});
}

export function getFetchPriorityProps(value?: FetchPriority) {
if (!value) {
return {};
}

const major = Number(React.version.split('.')[0]);

return major >= 19 ? { fetchPriority: value } : { fetchpriority: value };
}

// ============================= Legacy =============================
export function getClientSize() {
const width = document.documentElement.clientWidth;
Expand Down
41 changes: 41 additions & 0 deletions tests/fetchPriority.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
describe('getFetchPriorityProps', () => {
afterEach(() => {
jest.resetModules();
jest.dontMock('react');
});

it('uses lowercase fetchpriority for React 18', () => {
jest.doMock('react', () => ({
...jest.requireActual('react'),
version: '18.3.1',
}));

jest.isolateModules(() => {
const { getFetchPriorityProps } = require('../src/util');
expect(getFetchPriorityProps('high')).toEqual({
fetchpriority: 'high',
});
});
});

it('uses camelCase fetchPriority for React 19', () => {
jest.doMock('react', () => ({
...jest.requireActual('react'),
version: '19.2.4',
}));

jest.isolateModules(() => {
const { getFetchPriorityProps } = require('../src/util');
expect(getFetchPriorityProps('high')).toEqual({
fetchPriority: 'high',
});
});
});

it('returns empty props when value is undefined', () => {
jest.isolateModules(() => {
const { getFetchPriorityProps } = require('../src/util');
expect(getFetchPriorityProps()).toEqual({});
});
});
});
7 changes: 6 additions & 1 deletion tests/preview.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -809,11 +809,12 @@ describe('Preview', () => {
);
});

it('pass img common props to previewed image', () => {
it('passes image common props to previewed image', () => {
const { container } = render(
<Image
src="https://zos.alipayobjects.com/rmsportal/jkjgkEfvpUPVyRjUImniVslZfWPnJuuZ.png"
referrerPolicy="no-referrer"
fetchPriority="high"
/>,
);

Expand All @@ -826,6 +827,10 @@ describe('Preview', () => {
'referrerPolicy',
'no-referrer',
);
expect(document.querySelector('.rc-image-preview-img')).toHaveAttribute(
'fetchpriority',
'high',
);
});

describe('actionsRender', () => {
Expand Down