Skip to content

Commit

Permalink
fix: PreviewGroup async src order (#81)
Browse files Browse the repository at this point in the history
* fix: PreviewGroup async src order

* code format
  • Loading branch information
shaodahong authored Mar 18, 2021
1 parent 75a1764 commit d4e3473
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 58 deletions.
6 changes: 1 addition & 5 deletions src/Image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,7 @@ const ImageInternal: CompoundedComponent<ImageProps> = ({
}, []);

React.useEffect(() => {
const unRegister = registerImage(currentId, src);

if (!canPreview) {
unRegister();
}
registerImage(currentId, src, canPreview);
}, [src, canPreview]);
// Keep order end

Expand Down
33 changes: 20 additions & 13 deletions src/PreviewGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,20 @@ export interface GroupConsumerProps {
preview?: boolean | PreviewGroupPreview;
}

interface PreviewUrl {
url: string;
canPreview: boolean;
}

export interface GroupConsumerValue extends GroupConsumerProps {
isPreviewGroup?: boolean;
previewUrls: Map<number, string>;
setPreviewUrls: React.Dispatch<React.SetStateAction<Map<number, string>>>;
setPreviewUrls: React.Dispatch<React.SetStateAction<Map<number, PreviewUrl>>>;
current: number;
setCurrent: React.Dispatch<React.SetStateAction<number>>;
setShowPreview: React.Dispatch<React.SetStateAction<boolean>>;
setMousePosition: React.Dispatch<React.SetStateAction<null | { x: number; y: number }>>;
registerImage: (id: number, url: string) => () => void;
registerImage: (id: number, url: string, canPreview?: boolean) => () => void;
}

/* istanbul ignore next */
Expand Down Expand Up @@ -56,7 +61,7 @@ const Group: React.FC<GroupConsumerProps> = ({
current: currentIndex = 0,
...dialogProps
} = typeof preview === 'object' ? preview : {};
const [previewUrls, setPreviewUrls] = useState<Map<number, string>>(new Map());
const [previewUrls, setPreviewUrls] = useState<Map<number, PreviewUrl>>(new Map());
const [current, setCurrent] = useState<number>();
const [isShowPreview, setShowPreview] = useMergedState(!!previewVisible, {
value: previewVisible,
Expand All @@ -66,8 +71,13 @@ const Group: React.FC<GroupConsumerProps> = ({
const isControlled = previewVisible !== undefined;
const previewUrlsKeys = Array.from(previewUrls.keys());
const currentControlledKey = previewUrlsKeys[currentIndex];
const canPreviewUrls = new Map<number, string>(
Array.from(previewUrls)
.filter(([, { canPreview }]) => !!canPreview)
.map(([id, { url }]) => [id, url]),
);

const registerImage = (id: number, url: string) => {
const registerImage = (id: number, url: string, canPreview: boolean = true) => {
const unRegister = () => {
setPreviewUrls(oldPreviewUrls => {
const clonePreviewUrls = new Map(oldPreviewUrls);
Expand All @@ -76,14 +86,11 @@ const Group: React.FC<GroupConsumerProps> = ({
});
};

// we don't need to test this if canPreview changed when url stays the same
/* istanbul ignore next */
if (previewUrls.get(id) === url) {
return unRegister;
}

setPreviewUrls(oldPreviewUrls => {
return new Map(oldPreviewUrls).set(id, url);
return new Map(oldPreviewUrls).set(id, {
url,
canPreview,
});
});

return unRegister;
Expand All @@ -109,7 +116,7 @@ const Group: React.FC<GroupConsumerProps> = ({
<Provider
value={{
isPreviewGroup: true,
previewUrls,
previewUrls: canPreviewUrls,
setPreviewUrls,
current,
setCurrent,
Expand All @@ -125,7 +132,7 @@ const Group: React.FC<GroupConsumerProps> = ({
prefixCls={previewPrefixCls}
onClose={onPreviewClose}
mousePosition={mousePosition}
src={previewUrls.get(current)}
src={canPreviewUrls.get(current)}
icons={icons}
getContainer={getContainer}
{...dialogProps}
Expand Down
88 changes: 48 additions & 40 deletions tests/preview.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,7 @@ describe('Preview', () => {
});

act(() => {
wrapper
.find('.rc-image-preview-operations-operation')
.at(0)
.simulate('click');
wrapper.find('.rc-image-preview-operations-operation').at(0).simulate('click');
});

expect(onPreviewCloseMock).toBeCalledWith(false, true);
Expand Down Expand Up @@ -90,10 +87,7 @@ describe('Preview', () => {
});

act(() => {
wrapper
.find('.rc-image-preview-operations-operation')
.at(3)
.simulate('click');
wrapper.find('.rc-image-preview-operations-operation').at(3).simulate('click');
jest.runAllTimers();
wrapper.update();
});
Expand All @@ -102,10 +96,7 @@ describe('Preview', () => {
});

act(() => {
wrapper
.find('.rc-image-preview-operations-operation')
.at(4)
.simulate('click');
wrapper.find('.rc-image-preview-operations-operation').at(4).simulate('click');
jest.runAllTimers();
wrapper.update();
});
Expand All @@ -126,10 +117,7 @@ describe('Preview', () => {
});

act(() => {
wrapper
.find('.rc-image-preview-operations-operation')
.at(2)
.simulate('click');
wrapper.find('.rc-image-preview-operations-operation').at(2).simulate('click');
jest.runAllTimers();
wrapper.update();
});
Expand All @@ -138,10 +126,7 @@ describe('Preview', () => {
});

act(() => {
wrapper
.find('.rc-image-preview-operations-operation')
.at(1)
.simulate('click');
wrapper.find('.rc-image-preview-operations-operation').at(1).simulate('click');
jest.runAllTimers();
wrapper.update();
});
Expand All @@ -150,10 +135,7 @@ describe('Preview', () => {
});

act(() => {
wrapper
.find('.rc-image-preview-operations-operation')
.at(2)
.simulate('click');
wrapper.find('.rc-image-preview-operations-operation').at(2).simulate('click');
jest.runAllTimers();
wrapper.update();
});
Expand Down Expand Up @@ -404,39 +386,27 @@ describe('Preview', () => {
);

act(() => {
wrapper
.find('.rc-image')
.at(0)
.simulate('click');
wrapper.find('.rc-image').at(0).simulate('click');
jest.runAllTimers();
wrapper.update();
});

act(() => {
wrapper
.find('.anticon')
.at(1)
.simulate('click');
wrapper.find('.anticon').at(1).simulate('click');
jest.runAllTimers();
wrapper.update();
});

wrapper.find('.rc-image-preview-wrap').simulate('click');

act(() => {
wrapper
.find('.rc-image')
.at(1)
.simulate('click');
wrapper.find('.rc-image').at(1).simulate('click');
jest.runAllTimers();
wrapper.update();
});

act(() => {
wrapper
.find('.anticon')
.at(0)
.simulate('click');
wrapper.find('.anticon').at(0).simulate('click');
jest.runAllTimers();
wrapper.update();
});
Expand Down Expand Up @@ -510,4 +480,42 @@ describe('Preview', () => {
expect(wrapper.find('Preview').prop('transitionName')).toBe('abc');
expect(wrapper.find('Preview').prop('maskTransitionName')).toBe('def');
});

it('if async src set should be correct', () => {
const src =
'https://gw.alipayobjects.com/mdn/rms_08e378/afts/img/A*P0S-QIRUbsUAAAAAAAAAAABkARQnAQ';
const AsyncImage = ({ src: imgSrc }) => {
const normalSrc =
'https://zos.alipayobjects.com/rmsportal/jkjgkEfvpUPVyRjUImniVslZfWPnJuuZ.png';
return (
<Image.PreviewGroup>
<Image src={imgSrc} />
<Image src={normalSrc} />
</Image.PreviewGroup>
);
};

const wrapper = mount(<AsyncImage src="" />);

act(() => {
wrapper.setProps({
src,
});
});

act(() => {
wrapper.find('.rc-image').at(0).simulate('click');
});

jest.runAllTimers();
wrapper.update();

expect(wrapper.find('.rc-image-preview-img').at(0).prop('src')).toBe(src);

expect(
wrapper
.find('.rc-image-preview-switch-left')
.hasClass('rc-image-preview-switch-left-disabled'),
).toBe(true);
});
});

1 comment on commit d4e3473

@vercel
Copy link

@vercel vercel bot commented on d4e3473 Mar 18, 2021

Choose a reason for hiding this comment

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

Please sign in to comment.