Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Suspense for Image loading to reduce the cognitive overload in the code and also utilize the react18 concurrent rendering #632

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
2 changes: 2 additions & 0 deletions packages/react-sdk/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,14 @@
"react-beautiful-dnd": "^13.1.1",
"react-draggable": "^4.4.6",
"react-dropzone": "^14.2.9",
"react-error-boundary": "^4.1.2",
"react-hotkeys-hook": "^4.5.0",
"react-joyride": "^2.9.2",
"react-redux": "^9.1.2",
"react-transition-group": "^4.4.5",
"react-use": "^17.5.1",
"rxjs": "^7.8.1",
"swr": "^2.2.5",
"tinycolor2": "^1.6.0",
"yjs": "^13.6.19"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import { MockedWidgetApi, mockWidgetApi } from '@matrix-widget-toolkit/testing';
import { render, screen } from '@testing-library/react';
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import { ComponentType, PropsWithChildren } from 'react';
import {
MockInstance,
Expand All @@ -31,6 +31,7 @@ import {
mockImageElement,
mockWhiteboardManager,
} from '../../../lib/testUtils/documentTestUtils';
import { ImageMimeType } from '../../../state';
import { LayoutStateProvider } from '../../Layout';
import { SlidesProvider } from '../../Layout/SlidesProvider';
import { SvgCanvas } from '../SvgCanvas';
Expand Down Expand Up @@ -112,4 +113,123 @@ describe('<ConnectedElement />', () => {
screen.queryByTestId('element-element-0-image'),
).not.toBeInTheDocument();
});

it('should render an error when an image is not available', async () => {
vi.mocked(URL.createObjectURL).mockReturnValue('');

// @ts-ignore ignore readonly prop for tests
widgetApi.widgetParameters.baseUrl = 'https://example.com';

render(
<ConnectedElement
id="element-0"
activeElementIds={['element-0']}
readOnly={false}
overrides={{}}
/>,
{ wrapper: Wrapper },
);

expect(
screen.getByTestId('element-element-0-skeleton'),
).toBeInTheDocument();

const errorContainer = await screen.findByTestId(
'element-element-0-error-container',
);
expect(errorContainer).toBeInTheDocument();

const imageElement = screen.queryByTestId('element-element-0-image');
expect(imageElement).not.toBeInTheDocument();

vi.mocked(URL.createObjectURL).mockReset();
});

it.each([
['example.gif', 'image/gif'],
['example.jpeg', 'image/jpeg'],
['example.png', 'image/png'],
['example.svg', 'image/svg+xml'],
] as [string, ImageMimeType][])(
'should render %s without exploding',
async (fileName, mimeType) => {
widgetApi = mockWidgetApi();
consoleSpy = vi.spyOn(console, 'error');

vi.mocked(URL.createObjectURL).mockReturnValue('http://...');

const imageElementMock = mockImageElement();
imageElementMock.fileName = fileName;
imageElementMock.mimeType = mimeType;

const { whiteboardManager } = mockWhiteboardManager({
slides: [['slide-0', [['element-0', imageElementMock]]]],
});

Wrapper = ({ children }) => (
<LayoutStateProvider>
<WhiteboardTestingContextProvider
whiteboardManager={whiteboardManager}
widgetApi={widgetApi}
>
<SlidesProvider>
<SvgCanvas
viewportWidth={whiteboardWidth}
viewportHeight={whiteboardHeight}
>
{children}
</SvgCanvas>
</SlidesProvider>
</WhiteboardTestingContextProvider>
</LayoutStateProvider>
);

// @ts-ignore ignore readonly prop for tests
widgetApi.widgetParameters.baseUrl = 'https://example.com';

render(
<ConnectedElement
id="element-0"
activeElementIds={['element-0']}
readOnly={false}
overrides={{}}
/>,
{ wrapper: Wrapper },
);

expect(
screen.getByTestId('element-element-0-skeleton'),
).toBeInTheDocument();
const imageElement = await screen.findByTestId('element-element-0-image');
expect(imageElement).toBeInTheDocument();
},
);

it('should render a skeleton until an image is loaded', async () => {
// @ts-ignore ignore readonly prop for tests
widgetApi.widgetParameters.baseUrl = 'https://example.com';
render(
<ConnectedElement
id="element-0"
activeElementIds={['element-0']}
readOnly={false}
overrides={{}}
/>,
{ wrapper: Wrapper },
);

expect(
screen.getByTestId('element-element-0-skeleton'),
).toBeInTheDocument();

const imageElement = await screen.findByTestId('element-element-0-image');
expect(imageElement).toBeInTheDocument();
fireEvent.load(imageElement);

await waitFor(() => {
expect(
screen.queryByTestId('element-element-0-skeleton'),
).not.toBeInTheDocument();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,14 @@
*/

import { useWidgetApi } from '@matrix-widget-toolkit/react';
import { Suspense } from 'react';
import { ErrorBoundary } from 'react-error-boundary';
import { Elements } from '../../../state/types';
import { useElementOverride } from '../../ElementOverridesProvider';
import EllipseDisplay from '../../elements/ellipse/Display';
import ImageDisplay from '../../elements/image/ImageDisplay';
import { ImagePlaceholder } from '../../elements/image/ImagePlaceholder';
import { Skeleton } from '../../elements/image/Skeleton';
import LineDisplay from '../../elements/line/Display';
import PolylineDisplay from '../../elements/polyline/Display';
import RectangleDisplay from '../../elements/rectangle/Display';
Expand Down Expand Up @@ -72,11 +76,34 @@ export const ConnectedElement = ({
}

return (
<ImageDisplay
baseUrl={widgetApi.widgetParameters.baseUrl}
{...element}
{...otherProps}
/>
<Suspense
MTRNord marked this conversation as resolved.
Show resolved Hide resolved
fallback={
<Skeleton
data-testid={`element-${otherProps.elementId}-skeleton`}
x={element.position.x}
y={element.position.y}
width={element.width}
height={element.height}
/>
}
>
<ErrorBoundary
fallback={
<ImagePlaceholder
position={element.position}
width={element.width}
height={element.height}
elementId={otherProps.elementId}
/>
}
>
<ImageDisplay
baseUrl={widgetApi.widgetParameters.baseUrl}
{...element}
{...otherProps}
/>
</ErrorBoundary>
</Suspense>
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import { MockedWidgetApi, mockWidgetApi } from '@matrix-widget-toolkit/testing';
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import { render, screen } from '@testing-library/react';
import { ComponentType, PropsWithChildren } from 'react';
import {
afterEach,
Expand All @@ -31,7 +31,6 @@ import {
mockWhiteboardManager,
WhiteboardTestingContextProvider,
} from '../../../lib/testUtils/documentTestUtils';
import { ImageMimeType } from '../../../state';
import { LayoutStateProvider } from '../../Layout';
import { SlidesProvider } from '../../Layout/SlidesProvider';
import { whiteboardHeight, whiteboardWidth } from '../../Whiteboard';
Expand Down Expand Up @@ -86,107 +85,6 @@ describe('<ImageDisplay />', () => {
fetch.resetMocks();
});

it.each([
['example.gif', 'image/gif'],
['example.jpeg', 'image/jpeg'],
['example.png', 'image/png'],
['example.svg', 'image/svg+xml'],
] as [string, ImageMimeType][])(
'should render %s without exploding',
async (fileName, mimeType) => {
render(
<ImageDisplay
elementId="element-0"
type="image"
fileName={fileName}
mimeType={mimeType}
baseUrl="https://example.com"
mxc="mxc://example.com/test1234"
width={200}
height={300}
position={{ x: 23, y: 42 }}
active={false}
readOnly={false}
/>,
{ wrapper: Wrapper },
);

expect(
screen.getByTestId('element-element-0-skeleton'),
).toBeInTheDocument();
const imageElement = await screen.findByTestId('element-element-0-image');
expect(imageElement).toBeInTheDocument();
},
);

it('should render a skeleton until an image is loaded', async () => {
render(
<ImageDisplay
elementId="element-0"
type="image"
fileName="example.jpeg"
mimeType="image/jpeg"
baseUrl="https://example.com"
mxc="mxc://example.com/test1234"
width={200}
height={300}
position={{ x: 23, y: 42 }}
active={false}
readOnly={false}
/>,
{ wrapper: Wrapper },
);

expect(
screen.getByTestId('element-element-0-skeleton'),
).toBeInTheDocument();

const imageElement = await screen.findByTestId('element-element-0-image');
expect(imageElement).toBeInTheDocument();
fireEvent.load(imageElement);

await waitFor(() => {
expect(
screen.queryByTestId('element-element-0-skeleton'),
).not.toBeInTheDocument();
});
});

it('should render an error when an image is not available', async () => {
vi.mocked(URL.createObjectURL).mockReturnValue('');

render(
<ImageDisplay
elementId="element-0"
type="image"
fileName="example.jpeg"
mimeType="image/jpeg"
baseUrl="https://example.com"
mxc="mxc://example.com/test1234"
width={200}
height={300}
position={{ x: 23, y: 42 }}
active={false}
readOnly={false}
/>,
{ wrapper: Wrapper },
);

expect(
screen.getByTestId('element-element-0-skeleton'),
).toBeInTheDocument();

const errorContainer = await screen.findByTestId(
'element-element-0-error-container',
);
expect(errorContainer).toBeInTheDocument();

const imageElement = screen.queryByTestId('element-element-0-image');
expect(imageElement).not.toBeInTheDocument();

vi.mocked(URL.createObjectURL).mockReset();
});

it('should not have a context menu in read-only mode', () => {
render(
<ImageDisplay
Expand Down
Loading