Skip to content

Commit 180761a

Browse files
joshdimantetojoshdimanteto
committed
Address review comments #1081 
1 parent c36b1ff commit 180761a

File tree

11 files changed

+236
-174
lines changed

11 files changed

+236
-174
lines changed

‎cypress/e2e/with_mock_data/items.cy.ts

Lines changed: 8 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -701,40 +701,13 @@ describe('Items', () => {
701701
});
702702

703703
it('falls back to placeholder thumbnail', () => {
704-
cy.window().its('msw').should('not.equal', undefined);
705-
cy.window().then((window) => {
706-
const { worker, http } = window.msw;
707-
708-
worker.use(
709-
http.get('/images', async () => {
710-
return HttpResponse.json(
711-
[
712-
{
713-
id: '1',
714-
thumbnail_base64: 'test',
715-
file_name: 'test.png',
716-
},
717-
],
718-
{ status: 200 }
719-
);
720-
})
721-
);
722-
});
723-
724704
cy.findByText('5YUQDDjKpz2z').click();
725705
cy.findByText(
726706
'High-resolution cameras for beam characterization. 1'
727707
).should('exist');
728708

729709
cy.findByText('Gallery').click();
730-
731-
cy.findByRole('progressbar', { timeout: 10000 }).should('not.exist');
732-
733-
cy.findByRole('img', { timeout: 10000 }).should(
734-
'have.attr',
735-
'src',
736-
'http://localhost:3000/images/thumbnail-not-available.png'
737-
);
710+
cy.findByText('The image cannot be loaded').should('exist');
738711
});
739712

740713
it('opens full-size image when thumbnail is clicked and navigates to the next image', () => {
@@ -745,10 +718,10 @@ describe('Items', () => {
745718

746719
cy.findByText('Gallery').click();
747720

748-
cy.findAllByAltText('Image: tetstw').first().click();
721+
cy.findAllByAltText('Image: stfc-logo-blue-text').first().click();
749722

750723
cy.findByText('File Name: stfc-logo-blue-text.png').should('exist');
751-
cy.findByText('Title: tetstw').should('exist');
724+
cy.findByText('Title: stfc-logo-blue-text').should('exist');
752725
cy.findByText('test').should('exist');
753726

754727
cy.findByRole('dialog').within(() => {
@@ -758,7 +731,7 @@ describe('Items', () => {
758731
cy.findByRole('button', { name: 'Next' }).click();
759732

760733
cy.findByText('File Name: logo1.png').should('exist');
761-
cy.findByText('Title: tetstw').should('exist');
734+
cy.findByText('Title: logo1').should('exist');
762735
cy.findByText('test').should('exist');
763736

764737
cy.findByRole('dialog').within(() => {
@@ -768,11 +741,13 @@ describe('Items', () => {
768741
cy.findByRole('button', { name: 'Next' }).click();
769742
// Failed to render image
770743
cy.findByText('File Name: stfc-logo-blue-text.png').should('exist');
771-
cy.findByText('Title: tetstw').should('exist');
744+
cy.findByText('Title: stfc-logo-blue-text').should('exist');
772745
cy.findByText('test').should('exist');
773746

774747
cy.findByRole('dialog').within(() => {
775-
cy.findByRole('img', { timeout: 10000 }).should('exist');
748+
cy.findByText('The image cannot be loaded', { timeout: 15000 }).should(
749+
'exist'
750+
);
776751
});
777752
});
778753
});

‎public/images/image-not-available.png

-9.56 KB
Binary file not shown.

‎public/images/thumbnail-not-available.png

-4.33 KB
Binary file not shown.

‎src/common/images/__snapshots__/imageGallery.component.test.tsx.snap

Lines changed: 20 additions & 20 deletions
Large diffs are not rendered by default.

‎src/common/images/imageGallery.component.test.tsx

Lines changed: 11 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
1-
import { fireEvent, screen, waitFor, within } from '@testing-library/react';
1+
import { screen, waitFor, within } from '@testing-library/react';
22
import userEvent, { UserEvent } from '@testing-library/user-event';
33
import { http, HttpResponse } from 'msw';
44
import { act } from 'react';
55
import { MockInstance } from 'vitest';
66
import { storageApi } from '../../api/api';
7-
import ImageJSON from '../../mocks/image.json';
87
import { server } from '../../mocks/server';
98
import { renderComponentWithRouterProvider } from '../../testUtils';
109
import ImageGallery, { ImageGalleryProps } from './imageGallery.component';
@@ -117,45 +116,15 @@ describe('Image Gallery', () => {
117116
expect(baseElement).toMatchSnapshot();
118117
});
119118

120-
it('falls back to placeholder thumbnail', async () => {
121-
server.use(
122-
http.get('/images', async () => {
123-
return HttpResponse.json(
124-
[
125-
{
126-
...ImageJSON,
127-
id: '1',
128-
thumbnail_base64: 'test',
129-
file_name: 'test.png',
130-
},
131-
],
132-
{ status: 200 }
133-
);
134-
})
135-
);
136-
createView();
137-
138-
await waitFor(() =>
139-
expect(screen.queryByRole('progressbar')).not.toBeInTheDocument()
140-
);
141-
142-
const image = screen.getByRole('img') as HTMLImageElement; // Replace with actual alt text or selector
143-
144-
expect(image).toHaveAttribute('src', 'data:image/webp;base64,test');
145-
fireEvent.error(image);
146-
147-
await waitFor(() => {
148-
expect(image.src).toEqual('/images/thumbnail-not-available.png');
149-
});
150-
});
151-
152119
it('opens full-size image when thumbnail is clicked, navigates to the next image, and then navigates to a third image that failed to upload, falling back to a placeholder', async () => {
153120
createView();
154121

155122
await waitFor(() =>
156123
expect(screen.queryByRole('progressbar')).not.toBeInTheDocument()
157124
);
158-
const thumbnail = await screen.findAllByAltText('Image: tetstw');
125+
const thumbnail = await screen.findAllByAltText(
126+
'Image: stfc-logo-blue-text'
127+
);
159128
await user.click(thumbnail[0]);
160129

161130
expect(axiosGetSpy).toHaveBeenCalledWith('/images/1');
@@ -164,7 +133,7 @@ describe('Image Gallery', () => {
164133
screen.getByText('File Name: stfc-logo-blue-text.png')
165134
).toBeInTheDocument();
166135
});
167-
expect(screen.getByText('Title: tetstw')).toBeInTheDocument();
136+
expect(screen.getByText('Title: stfc-logo-blue-text')).toBeInTheDocument();
168137
expect(screen.getByText('test')).toBeInTheDocument();
169138

170139
await waitFor(
@@ -182,7 +151,7 @@ describe('Image Gallery', () => {
182151
await waitFor(() => {
183152
expect(screen.getByText('File Name: logo1.png')).toBeInTheDocument();
184153
});
185-
expect(screen.getByText('Title: tetstw')).toBeInTheDocument();
154+
expect(screen.getByText('Title: logo1')).toBeInTheDocument();
186155
expect(screen.getByText('test')).toBeInTheDocument();
187156

188157
await waitFor(
@@ -203,16 +172,11 @@ describe('Image Gallery', () => {
203172
screen.getByText('File Name: stfc-logo-blue-text.png')
204173
).toBeInTheDocument();
205174
});
206-
expect(screen.getByText('Title: tetstw')).toBeInTheDocument();
175+
expect(screen.getByText('Title: stfc-logo-blue-text')).toBeInTheDocument();
207176
expect(screen.getByText('test')).toBeInTheDocument();
208177

209-
await waitFor(
210-
() => {
211-
expect(
212-
within(screen.getByRole('dialog')).getByRole('img')
213-
).toBeInTheDocument();
214-
},
215-
{ timeout: 5000 }
216-
);
217-
}, 15000);
178+
await waitFor(() => {
179+
expect(axiosGetSpy).toHaveBeenCalledTimes(4);
180+
});
181+
}, 20000);
218182
});

‎src/common/images/imageGallery.component.tsx

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,8 @@ import 'photoswipe/dist/photoswipe.css';
1414
import React from 'react';
1515
import { Gallery, GalleryProps, Item } from 'react-photoswipe-gallery';
1616
import { getImage, useGetImages } from '../../api/images';
17-
import { InventoryManagementSystemSettingsContext } from '../../configProvider.component';
1817
import { OverflowTip } from '../../utils';
19-
import ImageNotAvailable from '/images/image-not-available.png';
20-
import ThumbnailNotAvailable from '/images/thumbnail-not-available.png';
18+
import ThumbnailImage from './thumbnailImage.component';
2119

2220
const MAX_HEIGHT_THUMBNAIL = 300;
2321

@@ -30,9 +28,6 @@ const ImageGallery = (props: ImageGalleryProps) => {
3028
const { data: images, isLoading: imageIsLoading } = useGetImages(entityId);
3129
const queryClient = useQueryClient();
3230

33-
const settings = React.useContext(InventoryManagementSystemSettingsContext);
34-
const pluginHost = settings.pluginHost;
35-
3631
const onBeforeOpen = React.useCallback(
3732
(pswpInstance: PhotoSwipe) => {
3833
let isFetching = false;
@@ -79,9 +74,7 @@ const ImageGallery = (props: ImageGalleryProps) => {
7974
pswpInstance.refreshSlideContent(pswpInstance.currIndex);
8075
} catch {
8176
Object.assign(slide, {
82-
src: pluginHost + ImageNotAvailable,
83-
width: 696,
84-
height: 525,
77+
src: 'data:image/jpeg;base64,invalidBase64data',
8578
});
8679
pswpInstance.refreshSlideContent(pswpInstance.currIndex);
8780
} finally {
@@ -95,7 +88,7 @@ const ImageGallery = (props: ImageGalleryProps) => {
9588
pswpInstance.off('slideInit', slideInitHandler);
9689
};
9790
},
98-
[pluginHost, queryClient]
91+
[queryClient]
9992
);
10093

10194
const options: GalleryProps['options'] = {
@@ -190,23 +183,12 @@ const ImageGallery = (props: ImageGalleryProps) => {
190183
>
191184
{({ ref, open }) => {
192185
return (
193-
<Box
186+
<ThumbnailImage
194187
ref={ref}
195-
component="img"
196-
src={`data:image/webp;base64,${image.thumbnail_base64}`}
197-
alt={`Image: ${image.title || image.file_name || index}`}
198-
style={{
199-
maxWidth: `${MAX_HEIGHT_THUMBNAIL}px`,
200-
maxHeight: `${MAX_HEIGHT_THUMBNAIL}px`,
201-
borderRadius: '4px',
202-
cursor: 'pointer',
203-
}}
204-
onClick={open}
205-
onError={(e) => {
206-
const target = e.target as HTMLImageElement;
207-
target.onerror = null; // Prevent looping
208-
target.src = pluginHost + ThumbnailNotAvailable;
209-
}}
188+
open={open}
189+
image={image}
190+
maxHeightThumbnail={MAX_HEIGHT_THUMBNAIL}
191+
index={index}
210192
/>
211193
);
212194
}}

‎src/common/images/thumbnailImage.component.test.tsx

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import '@testing-library/jest-dom';
2+
import { fireEvent, render, screen } from '@testing-library/react';
3+
import React from 'react';
4+
import ImagesJSON from '../../mocks/Images.json';
5+
import { renderComponentWithRouterProvider } from '../../testUtils';
6+
import ThumbnailImage, {
7+
ThumbnailImageProps,
8+
} from './thumbnailImage.component';
9+
10+
describe('ThumbnailImage Component', () => {
11+
let props: ThumbnailImageProps;
12+
const mockOpen = vi.fn();
13+
14+
const createView = () => {
15+
renderComponentWithRouterProvider(<ThumbnailImage {...props} />);
16+
};
17+
18+
beforeEach(() => {
19+
props = {
20+
open: mockOpen,
21+
image: ImagesJSON[0],
22+
index: 0,
23+
maxHeightThumbnail: 100,
24+
};
25+
});
26+
27+
afterEach(() => {
28+
vi.clearAllMocks();
29+
});
30+
it('should render an image when the thumbnail loads successfully', () => {
31+
createView();
32+
33+
const imageElement = screen.getByAltText(`Image: ${props.image.title}`);
34+
expect(imageElement).toBeInTheDocument();
35+
expect(imageElement).toHaveAttribute(
36+
'src',
37+
`data:image/webp;base64,${props.image.thumbnail_base64}`
38+
);
39+
});
40+
41+
it('should render fallback text when the image fails to load', () => {
42+
props.image.thumbnail_base64 = 'test';
43+
createView();
44+
45+
const imageElement = screen.getByAltText(`Image: ${props.image.title}`);
46+
fireEvent.error(imageElement);
47+
48+
const fallbackText = screen.getByText('The image cannot be loaded');
49+
expect(fallbackText).toBeInTheDocument();
50+
});
51+
52+
it('should call the open function when the image is clicked', () => {
53+
createView();
54+
55+
const imageElement = screen.getByAltText(`Image: ${props.image.title}`);
56+
fireEvent.click(imageElement);
57+
58+
expect(mockOpen).toHaveBeenCalledTimes(1);
59+
});
60+
61+
it('should call the open function when the fallback text is clicked', () => {
62+
createView();
63+
64+
const imageElement = screen.getByAltText(`Image: ${props.image.title}`);
65+
fireEvent.error(imageElement);
66+
67+
const fallbackText = screen.getByText('The image cannot be loaded');
68+
fireEvent.click(fallbackText);
69+
70+
expect(mockOpen).toHaveBeenCalledTimes(1);
71+
});
72+
73+
it('should forward the ref to the image element', () => {
74+
const ref = React.createRef<HTMLElement>();
75+
render(<ThumbnailImage {...props} ref={ref} />);
76+
77+
// Check if the ref is assigned to the image
78+
const imageElement = screen.getByAltText(`Image: ${props.image.title}`);
79+
expect(ref.current).toBe(imageElement);
80+
});
81+
});

‎src/common/images/thumbnailImage.component.tsx

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { Box, Typography } from '@mui/material';
2+
import React from 'react';
3+
import { Image } from '../../api/api.types';
4+
5+
export interface ThumbnailImageProps {
6+
open: (e: React.MouseEvent) => void;
7+
image: Image;
8+
index: number;
9+
maxHeightThumbnail: number;
10+
}
11+
12+
const ThumbnailImage = React.forwardRef<HTMLElement, ThumbnailImageProps>(
13+
(props, ref) => {
14+
const { open, image, index, maxHeightThumbnail } = props;
15+
const [hasError, setHasError] = React.useState(false);
16+
17+
return (
18+
<>
19+
{hasError ? (
20+
<Typography
21+
maxWidth={`${maxHeightThumbnail}px`}
22+
maxHeight={`${maxHeightThumbnail}px`}
23+
variant="body2"
24+
color="textSecondary"
25+
textAlign="center"
26+
onClick={open}
27+
ref={ref}
28+
sx={{ cursor: 'pointer' }}
29+
>
30+
The image cannot be loaded
31+
</Typography>
32+
) : (
33+
<Box
34+
ref={ref}
35+
component="img"
36+
src={`data:image/webp;base64,${image.thumbnail_base64}`}
37+
alt={`Image: ${image.title || image.file_name || index}`}
38+
style={{
39+
maxWidth: `${maxHeightThumbnail}px`,
40+
maxHeight: `${maxHeightThumbnail}px`,
41+
borderRadius: '4px',
42+
cursor: 'pointer',
43+
}}
44+
onClick={open}
45+
onError={() => setHasError(true)}
46+
/>
47+
)}
48+
</>
49+
);
50+
}
51+
);
52+
53+
ThumbnailImage.displayName = 'ThumbnailImage';
54+
55+
export default ThumbnailImage;

0 commit comments

Comments
 (0)