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

Commit

Permalink
Fix some image/video scroll jumps (#8182)
Browse files Browse the repository at this point in the history 
* Fix some image/video scroll jumps

* Fix aspect ratio formatting

* Fix videos not being responsive to timeline width
  • Loading branch information
@robintown
robintown authored Mar 28, 2022
1 parent afa60ac commit af6bd63
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 85 deletions.
42 changes: 22 additions & 20 deletions res/css/views/messages/_MImageBody.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,40 +17,42 @@ limitations under the License.

$timeline-image-border-radius: 8px;

.mx_MImageBody_thumbnail--blurhash {
.mx_MImageBody_placeholder {
// Position the placeholder on top of the thumbnail, so that the reveal animation can work
position: absolute;
left: 0;
top: 0;
}

.mx_MImageBody_thumbnail {
object-fit: contain;
border-radius: $timeline-image-border-radius;

display: flex;
justify-content: center;
align-items: center;
height: 100%;
width: 100%;

// this is needed so that the Blurhash can get have rounded corners without beeing the correct size during loading.
overflow: hidden;
background-color: $background;

.mx_Blurhash > canvas {
animation: mx--anim-pulse 1.75s infinite cubic-bezier(.4, 0, .6, 1);
}

.mx_no-image-placeholder {
background-color: $primary-content;
}
}

.mx_MImageBody_thumbnail_container {
// Prevent the padding-bottom (added inline in MImageBody.js) from
// affecting elements below the container.
border-radius: $timeline-image-border-radius;

// Necessary for the border radius to apply correctly to the placeholder
overflow: hidden;
contain: paint;
}

// Make sure the _thumbnail is positioned relative to the _container
position: relative;
.mx_MImageBody_thumbnail {
display: block;

// Force the image to be the full size of the container, even if the
// pixel size is smaller. The problem here is that we don't know what
// thumbnail size the HS is going to give us, but we have to commit to
// a container size immediately and not change it when the image loads
// or we'll get a scroll jump (or have to leave blank space).
// This will obviously result in an upscaled image which will be a bit
// blurry. The best fix would be for the HS to advertise what size thumbnails
// it guarantees to produce.
height: 100%;
width: 100%;
}

.mx_MImageBody_gifLabel {
Expand Down
10 changes: 9 additions & 1 deletion res/css/views/messages/_MVideoBody.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,15 @@ limitations under the License.
*/

span.mx_MVideoBody {
video.mx_MVideoBody {
overflow: hidden;

.mx_MVideoBody_container {
border-radius: $timeline-image-border-radius;
overflow: hidden;

video {
height: 100%;
width: 100%;
}
}
}
21 changes: 14 additions & 7 deletions res/css/views/rooms/_EventBubbleTile.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,9 @@ limitations under the License.
.mx_EventTile_line {
border-bottom-right-radius: var(--cornerRadius);

.mx_MImageBody .mx_MImageBody_thumbnail,
.mx_MImageBody .mx_MImageBody_thumbnail_container,
.mx_MImageBody::before,
.mx_MVideoBody .mx_MVideoBody_container,
.mx_MediaBody,
.mx_MLocationBody_map {
border-bottom-right-radius: var(--cornerRadius) !important;
Expand All @@ -150,8 +151,9 @@ limitations under the License.
float: right;
border-bottom-left-radius: var(--cornerRadius);

.mx_MImageBody .mx_MImageBody_thumbnail,
.mx_MImageBody .mx_MImageBody_thumbnail_container,
.mx_MImageBody::before,
.mx_MVideoBody .mx_MVideoBody_container,
.mx_MediaBody,
.mx_MLocationBody_map {
border-bottom-left-radius: var(--cornerRadius) !important;
Expand Down Expand Up @@ -266,7 +268,8 @@ limitations under the License.
}

//noinspection CssReplaceWithShorthandSafely
.mx_MImageBody .mx_MImageBody_thumbnail,
.mx_MImageBody .mx_MImageBody_thumbnail_container,
.mx_MVideoBody .mx_MVideoBody_container,
.mx_MediaBody {
border-radius: unset;
border-top-left-radius: var(--cornerRadius);
Expand All @@ -293,7 +296,8 @@ limitations under the License.
&.mx_EventTile_continuation[data-self=false] .mx_EventTile_line {
border-top-left-radius: 0;

.mx_MImageBody .mx_MImageBody_thumbnail,
.mx_MImageBody .mx_MImageBody_thumbnail_container,
.mx_MVideoBody .mx_MVideoBody_container,
.mx_MImageBody::before,
.mx_MediaBody,
.mx_MLocationBody_map {
Expand All @@ -303,7 +307,8 @@ limitations under the License.
&.mx_EventTile_lastInSection[data-self=false] .mx_EventTile_line {
border-bottom-left-radius: var(--cornerRadius);

.mx_MImageBody .mx_MImageBody_thumbnail,
.mx_MImageBody .mx_MImageBody_thumbnail_container,
.mx_MVideoBody .mx_MVideoBody_container,
.mx_MImageBody::before,
.mx_MediaBody,
.mx_MLocationBody_map {
Expand All @@ -314,7 +319,8 @@ limitations under the License.
&.mx_EventTile_continuation[data-self=true] .mx_EventTile_line {
border-top-right-radius: 0;

.mx_MImageBody .mx_MImageBody_thumbnail,
.mx_MImageBody .mx_MImageBody_thumbnail_container,
.mx_MVideoBody .mx_MVideoBody_container,
.mx_MImageBody::before,
.mx_MediaBody,
.mx_MLocationBody_map {
Expand All @@ -324,7 +330,8 @@ limitations under the License.
&.mx_EventTile_lastInSection[data-self=true] .mx_EventTile_line {
border-bottom-right-radius: var(--cornerRadius);

.mx_MImageBody .mx_MImageBody_thumbnail,
.mx_MImageBody .mx_MImageBody_thumbnail_container,
.mx_MVideoBody .mx_MVideoBody_container,
.mx_MImageBody::before,
.mx_MediaBody,
.mx_MLocationBody_map {
Expand Down
51 changes: 14 additions & 37 deletions src/components/views/messages/MImageBody.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import MFileBody from './MFileBody';
import Modal from '../../../Modal';
import { _t } from '../../../languageHandler';
import SettingsStore from "../../../settings/SettingsStore";
import InlineSpinner from '../elements/InlineSpinner';
import Spinner from '../elements/Spinner';
import { replaceableComponent } from "../../../utils/replaceableComponent";
import { Media, mediaFromContent } from "../../../customisations/Media";
import { BLURHASH_FIELD, createThumbnail } from "../../../ContentMessages";
Expand Down Expand Up @@ -427,15 +427,6 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {
className="mx_MImageBody_thumbnail"
src={thumbUrl}
ref={this.image}
// Force the image to be the full size of the container, even if the
// pixel size is smaller. The problem here is that we don't know what
// thumbnail size the HS is going to give us, but we have to commit to
// a container size immediately and not change it when the image loads
// or we'll get a scroll jump (or have to leave blank space).
// This will obviously result in an upscaled image which will be a bit
// blurry. The best fix would be for the HS to advertise what size thumbnails
// it guarantees to produce.
style={{ height: '100%' }}
alt={content.body}
onError={this.onImageError}
onLoad={this.onImageLoad}
Expand All @@ -456,44 +447,32 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {
}

const classes = classNames({
'mx_MImageBody_thumbnail': true,
'mx_MImageBody_thumbnail--blurhash': this.props.mxEvent.getContent().info?.[BLURHASH_FIELD],
'mx_MImageBody_placeholder': true,
'mx_MImageBody_placeholder--blurhash': this.props.mxEvent.getContent().info?.[BLURHASH_FIELD],
});

// This has incredibly broken types.
const C = CSSTransition as any;
const thumbnail = (
<div className="mx_MImageBody_thumbnail_container" style={{ maxHeight, maxWidth, aspectRatio: `${infoWidth}/${infoHeight}` }}>
<SwitchTransition mode="out-in">
<C
<CSSTransition
classNames="mx_rtg--fade"
key={`img-${showPlaceholder}`}
timeout={300}
>
{ /* This weirdly looking div is necessary here, otherwise SwitchTransition fails */ }
<div>
{ showPlaceholder && <div
className={classes}
style={{
// Constrain width here so that spinner appears central to the loaded thumbnail
maxWidth,
maxHeight,
aspectRatio: `${infoWidth}/${infoHeight}`,
}}
>
{ placeholder }
</div> }
</div>
</C>
{ showPlaceholder ? <div className={classes}>
{ placeholder }
</div> : <></> /* Transition always expects a child */ }
</CSSTransition>
</SwitchTransition>

<div style={{
height: '100%',
}}>
<div style={{ maxHeight, maxWidth }}>
{ img }
{ gifLabel }
</div>

{ /* HACK: This div fills out space while the image loads, to prevent scroll jumps */ }
{ !this.state.imgLoaded && <div style={{ height: maxHeight, width: maxWidth }} /> }

{ this.state.hover && this.getTooltip() }
</div>
);
Expand All @@ -514,14 +493,12 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {

if (blurhash) {
if (this.state.placeholder === Placeholder.NoImage) {
return <div className="mx_no-image-placeholder" style={{ width: width, height: height }} />;
return null;
} else if (this.state.placeholder === Placeholder.Blurhash) {
return <Blurhash className="mx_Blurhash" hash={blurhash} width={width} height={height} />;
}
}
return (
<InlineSpinner w={32} h={32} />
);
return <Spinner w={32} h={32} />;
}

// Overidden by MStickerBody
Expand Down
50 changes: 30 additions & 20 deletions src/components/views/messages/MVideoBody.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,18 @@ export default class MVideoBody extends React.PureComponent<IBodyProps, IState>
const content = this.props.mxEvent.getContent();
const autoplay = SettingsStore.getValue("autoplayVideo");

let aspectRatio;
if (content.info?.w && content.info?.h) {
aspectRatio = `${content.info.w}/${content.info.h}`;
}
const { w: maxWidth, h: maxHeight } = suggestedVideoSize(
SettingsStore.getValue("Images.size") as ImageSize,
{ w: content.info?.w, h: content.info?.h },
);

// HACK: This div fills out space while the video loads, to prevent scroll jumps
const spaceFiller = <div style={{ width: maxWidth, height: maxHeight }} />;

if (this.state.error !== null) {
return (
<span className="mx_MVideoBody">
Expand All @@ -241,21 +253,17 @@ export default class MVideoBody extends React.PureComponent<IBodyProps, IState>
if (!this.props.forExport && content.file !== undefined && this.state.decryptedUrl === null && autoplay) {
// Need to decrypt the attachment
// The attachment is decrypted in componentDidMount.
// For now add an img tag with a spinner.
// For now show a spinner.
return (
<span className="mx_MVideoBody">
<div className="mx_MImageBody_thumbnail mx_MImageBody_thumbnail_spinner">
<div className="mx_MVideoBody_container" style={{ maxWidth, maxHeight, aspectRatio }}>
<InlineSpinner />
</div>
{ spaceFiller }
</span>
);
}

const { w: maxWidth, h: maxHeight } = suggestedVideoSize(
SettingsStore.getValue("Images.size") as ImageSize,
{ w: content.info?.w, h: content.info?.h },
);

const contentUrl = this.getContentUrl();
const thumbUrl = this.getThumbUrl();
let poster = null;
Expand All @@ -268,19 +276,21 @@ export default class MVideoBody extends React.PureComponent<IBodyProps, IState>
const fileBody = this.getFileBody();
return (
<span className="mx_MVideoBody">
<video
className="mx_MVideoBody"
ref={this.videoRef}
src={contentUrl}
title={content.body}
controls
preload={preload}
muted={autoplay}
autoPlay={autoplay}
style={{ maxHeight, maxWidth }}
poster={poster}
onPlay={this.videoOnPlay}
/>
<div className="mx_MVideoBody_container" style={{ maxWidth, maxHeight, aspectRatio }}>
<video
className="mx_MVideoBody"
ref={this.videoRef}
src={contentUrl}
title={content.body}
controls
preload={preload}
muted={autoplay}
autoPlay={autoplay}
poster={poster}
onPlay={this.videoOnPlay}
/>
{ spaceFiller }
</div>
{ fileBody }
</span>
);
Expand Down

0 comments on commit af6bd63

Please sign in to comment.