Fix some image/video scroll jumps (#8182)

robintown · web-flow · commit af6bd63ac7ab · 2022-03-28T19:07:02.000Z
* Fix some image/video scroll jumps

* Fix aspect ratio formatting

* Fix videos not being responsive to timeline width
diff --git a/res/css/views/messages/_MImageBody.scss b/res/css/views/messages/_MImageBody.scss
@@ -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 {
diff --git a/res/css/views/messages/_MVideoBody.scss b/res/css/views/messages/_MVideoBody.scss
@@ -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%;
+        }
     }
 }
diff --git a/res/css/views/rooms/_EventBubbleTile.scss b/res/css/views/rooms/_EventBubbleTile.scss
@@ -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;
@@ -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;
@@ -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);
@@ -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 {
@@ -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 {
@@ -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 {
@@ -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 {
diff --git a/src/components/views/messages/MImageBody.tsx b/src/components/views/messages/MImageBody.tsx
@@ -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";
@@ -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}
@@ -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>
         );
@@ -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
diff --git a/src/components/views/messages/MVideoBody.tsx b/src/components/views/messages/MVideoBody.tsx
@@ -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">
@@ -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;
@@ -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>
         );
