Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix: Images no longer reserve their space in the timeline correctly #10571

Merged
merged 7 commits into from
Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
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
12 changes: 6 additions & 6 deletions src/components/views/messages/MImageBody.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -385,22 +385,22 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {
}

protected messageContent(
contentUrl: string,
contentUrl: string | null,
thumbUrl: string | null,
content: IMediaEventContent,
forcedHeight?: number,
): JSX.Element {
if (!thumbUrl) thumbUrl = contentUrl; // fallback

let infoWidth: number;
let infoHeight: number;
let infoWidth = Number.MAX_SAFE_INTEGER;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this not better be set to a fallback of 🤷 50px for good measure? If the conditions below don't work there will be a huuuge thumbnail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is clamped down later by suggestedImageSize to maxHeight maxWidth, then these values are only used to set aspect ratio. I've changed it to a safer magic number

let infoHeight = Number.MAX_SAFE_INTEGER;
let infoSvg = false;

if (content.info?.w && content.info?.h) {
infoWidth = content.info.w;
infoHeight = content.info.h;
infoSvg = content.info.mimetype === "image/svg+xml";
} else {
} else if (thumbUrl && contentUrl) {
// Whilst the image loads, display nothing. We also don't display a blurhash image
// because we don't really know what size of image we'll end up with.
//
Expand Down Expand Up @@ -522,7 +522,7 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {
</div>
);

return this.wrapImage(contentUrl, thumbnail);
return contentUrl ? this.wrapImage(contentUrl, thumbnail) : thumbnail;
}

// Overridden by MStickerBody
Expand Down Expand Up @@ -596,7 +596,7 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {
thumbUrl = this.state.thumbUrl ?? this.state.contentUrl;
}

const thumbnail = contentUrl ? this.messageContent(contentUrl, thumbUrl, content) : undefined;
const thumbnail = this.messageContent(contentUrl, thumbUrl, content);
const fileBody = this.getFileBody();

return (
Expand Down
19 changes: 19 additions & 0 deletions test/components/views/messages/MImageBody-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ describe("<MImageBody/>", () => {
sender: userId,
type: EventType.RoomMessage,
content: {
info: {
w: 40,
h: 50,
},
file: {
url: "mxc://server/encrypted-image",
},
Expand All @@ -72,6 +76,21 @@ describe("<MImageBody/>", () => {
permalinkCreator: new RoomPermalinkCreator(new Room(encryptedMediaEvent.getRoomId()!, cli, cli.getUserId()!)),
};

it("should show a thumbnail while image is being downloaded", async () => {
fetchMock.getOnce(url, { status: 200 });

const { container } = render(
<MImageBody
{...props}
mxEvent={encryptedMediaEvent}
mediaEventHelper={new MediaEventHelper(encryptedMediaEvent)}
/>,
);

// thumbnail with dimensions present
expect(container).toMatchSnapshot();
});

it("should show error when encrypted media cannot be downloaded", async () => {
fetchMock.getOnce(url, { status: 500 });

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`<MImageBody/> should show a thumbnail while image is being downloaded 1`] = `
<div>
<div
class="mx_MImageBody"
>
<div
class="mx_MImageBody_thumbnail_container"
style="max-height: 50px; max-width: 40px;"
>
<div
class="mx_MImageBody_placeholder"
>
<div
class="mx_Spinner"
>
<div
aria-label="Loading…"
class="mx_Spinner_icon"
data-testid="spinner"
role="progressbar"
style="width: 32px; height: 32px;"
/>
</div>
</div>
<div
style="max-height: 50px; max-width: 40px;"
/>
<div
style="height: 50px; width: 40px;"
/>
</div>
</div>
</div>
`;