Implement slightly magical CSS soln. to thumbnail sizing

res/css/views/messages/_MImageBody.scss

@@ -20,5 +20,29 @@ limitations under the License.
 }
 
 .mx_MImageBody_thumbnail {
-    max-width: 100%;
-}
+    position: absolute;
+    width: 100%;
+    height: 100%;
+    left: 0;
+    top: 0;
+}
+
+.mx_MImageBody_thumbnail_container {
+    // Prevent the padding-bottom (added inline in MImageBody.js) from
+    // affecting elements below the container.
+    overflow: hidden;
+
+    // Make sure the _thumbnail is positioned relative to the _container
+    position: relative;
+}
+
+.mx_MImageBody_thumbnail_spinner {
+    position: absolute;
+    left: 50%;
+    top: 50%;
+}
+
+// Inner img and TintableSvg should be centered around 0, 0
+.mx_MImageBody_thumbnail_spinner > * {
+    transform: translate(-50%, -50%);
+}

res/css/views/messages/_MStickerBody.scss

@@ -14,33 +14,11 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-.mx_MStickerBody {
-  display: block;
-  margin-right: 34px;
-  min-height: 110px;
-  padding: 20px 0;
+.mx_MStickerBody_wrapper {
+    padding: 20px 0px;
 }
 
-.mx_MStickerBody_image_container {
-  display: inline-block;
-  position: relative;
-}
-
-.mx_MStickerBody_image {
-  max-width: 100%;
-  opacity: 0;
-}
-
-.mx_MStickerBody_image_visible {
-  opacity: 1;
-}
-
-.mx_MStickerBody_placeholder {
-  position: absolute;
-  opacity: 1;
-}
-
-.mx_MStickerBody_placeholder_invisible {
-  transition: 500ms;
-  opacity: 0;
+.mx_MStickerBody_tooltip {
+    position: absolute;
+    top: 50%;
 }

src/components/views/messages/MImageBody.js

@@ -22,15 +22,15 @@ import PropTypes from 'prop-types';
 import { MatrixClient } from 'matrix-js-sdk';
 
 import MFileBody from './MFileBody';
-import ImageUtils from '../../../ImageUtils';
 import Modal from '../../../Modal';
 import sdk from '../../../index';
-import dis from '../../../dispatcher';
 import { decryptFile } from '../../../utils/DecryptFile';
 import Promise from 'bluebird';
 import { _t } from '../../../languageHandler';
 import SettingsStore from "../../../settings/SettingsStore";
 
+const THUMBNAIL_MAX_HEIGHT = 600;
+
 export default class extends React.Component {
     displayName: 'MImageBody'
 
@@ -49,14 +49,12 @@ export default class extends React.Component {
     constructor(props) {
         super(props);
 
-        this.onAction = this.onAction.bind(this);
         this.onImageError = this.onImageError.bind(this);
         this.onImageLoad = this.onImageLoad.bind(this);
         this.onImageEnter = this.onImageEnter.bind(this);
         this.onImageLeave = this.onImageLeave.bind(this);
         this.onClientSync = this.onClientSync.bind(this);
         this.onClick = this.onClick.bind(this);
-        this.fixupHeight = this.fixupHeight.bind(this);
         this._isGif = this._isGif.bind(this);
 
         this.state = {
@@ -65,6 +63,8 @@ export default class extends React.Component {
             decryptedBlob: null,
             error: null,
             imgError: false,
+            imgLoaded: false,
+            hover: false,
         };
     }
 
@@ -119,6 +119,8 @@ export default class extends React.Component {
     }
 
     onImageEnter(e) {
+        this.setState({ hover: true });
+
         if (!this._isGif() || SettingsStore.getValue("autoplayGifsAndVideos")) {
             return;
         }
@@ -127,6 +129,8 @@ export default class extends React.Component {
     }
 
     onImageLeave(e) {
+        this.setState({ hover: false });
+
         if (!this._isGif() || SettingsStore.getValue("autoplayGifsAndVideos")) {
             return;
         }
@@ -142,6 +146,7 @@ export default class extends React.Component {
 
     onImageLoad() {
         this.props.onWidgetLoad();
+        this.setState({ imgLoaded: true });
     }
 
     _getContentUrl() {
@@ -176,7 +181,6 @@ export default class extends React.Component {
     }
 
     componentDidMount() {
-        this.dispatcherRef = dis.register(this.onAction);
         const content = this.props.mxEvent.getContent();
         if (content.file !== undefined && this.state.decryptedUrl === null) {
             let thumbnailPromise = Promise.resolve(null);
@@ -207,7 +211,6 @@ export default class extends React.Component {
                 });
             }).done();
         }
-        this.fixupHeight();
         this._afterComponentDidMount();
     }
 
@@ -218,7 +221,6 @@ export default class extends React.Component {
 
     componentWillUnmount() {
         this.unmounted = true;
-        dis.unregister(this.dispatcherRef);
         this.context.matrixClient.removeListener('sync', this.onClientSync);
         this._afterComponentWillUnmount();
 
@@ -235,59 +237,87 @@ export default class extends React.Component {
     _afterComponentWillUnmount() {
     }
 
-    onAction(payload) {
-        if (payload.action === "timeline_resize") {
-            this.fixupHeight();
+    _messageContent(contentUrl, thumbUrl, content) {
+        // The maximum height of the thumbnail as it is rendered as an <img>
+        const maxHeight = Math.min(THUMBNAIL_MAX_HEIGHT, content.info.h);
+        // The maximum width of the thumbnail, as dictated by its natural
+        // maximum height.
+        const maxWidth = content.info.w * maxHeight / content.info.h;
+
+        let img = null;
+        let placeholder = null;
+
+        // e2e image hasn't been decrypted yet
+        if (content.file !== undefined && this.state.decryptedUrl === null) {
+            placeholder = <img src="img/spinner.gif" alt={content.body} width="32" height="32" />;
+        } else if (!this.state.imgLoaded) {
+            // Deliberately, getSpinner is left unimplemented here, MStickerBody overides
+            placeholder = this.getPlaceholder();
         }
-    }
 
-    fixupHeight() {
-        if (!this.refs.image) {
-            console.warn(`Refusing to fix up height on ${this.displayName} with no image element`);
-            return;
+        const showPlaceholder = Boolean(placeholder);
+
+        if (thumbUrl && !this.state.imgError) {
+            // Restrict the width of the thumbnail here, otherwise it will fill the container
+            // which has the same width as the timeline
+            // mx_MImageBody_thumbnail resizes img to exactly container size
+            img = <img className="mx_MImageBody_thumbnail" src={thumbUrl} ref="image"
+                style={{ "max-width": maxWidth + "px" }}
+                alt={content.body}
+                onError={this.onImageError}
+                onLoad={this.onImageLoad}
+                onMouseEnter={this.onImageEnter}
+                onMouseLeave={this.onImageLeave} />;
         }
 
-        const content = this.props.mxEvent.getContent();
-        const timelineWidth = this.refs.body.offsetWidth;
-        const maxHeight = 600; // let images take up as much width as they can so long as the height doesn't exceed 600px.
-        // the alternative here would be 600*timelineWidth/800; to scale them down to fit inside a 4:3 bounding box
+        const thumbnail = (
+            <div className="mx_MImageBody_thumbnail_container" style={{ "max-height": maxHeight + "px" }} >
+                { /* Calculate aspect ratio, using %padding will size _container correctly */ }
+                <div style={{ paddingBottom: (100 * content.info.h / content.info.w) + '%' }}></div>
+
+                { showPlaceholder &&
+                    <div className="mx_MImageBody_thumbnail" style={{
+                        // Constrain width here so that spinner appears central to the loaded thumbnail
+                        "max-width": content.info.w + "px",
+                    }}>
+                        <div className="mx_MImageBody_thumbnail_spinner">
+                            { placeholder }
+                        </div>
+                    </div>
+                }
 
-        // FIXME: this will break on clientside generated thumbnails (as per e2e rooms)
-        // which may well be much smaller than the 800x600 bounding box.
+                <div style={{display: !showPlaceholder ? undefined : 'none'}}>
+                    { img }
+                </div>
 
-        // FIXME: It will also break really badly for images with broken or missing thumbnails
+                { this.state.hover && this.getTooltip() }
+            </div>
+        );
 
-        // FIXME: Because we don't know what size of thumbnail the server's actually going to send
-        // us, we can't even really layout the page nicely for it.  Instead we have to assume
-        // it'll target 800x600 and we'll downsize if needed to make things fit.
+        return this.wrapImage(contentUrl, thumbnail);
+    }
 
-        // console.log("trying to fit image into timelineWidth of " + this.refs.body.offsetWidth + " or " + this.refs.body.clientWidth);
-        let thumbHeight = null;
-        if (content.info) {
-            thumbHeight = ImageUtils.thumbHeight(content.info.w, content.info.h, timelineWidth, maxHeight);
-        }
-        this.refs.image.style.height = thumbHeight + "px";
-        // console.log("Image height now", thumbHeight);
+    // Overidden by MStickerBody
+    wrapImage(contentUrl, children) {
+        return <a href={contentUrl} onClick={this.onClick}>
+            {children}
+        </a>;
     }
 
-    _messageContent(contentUrl, thumbUrl, content) {
-        const thumbnail = (
-            <a href={contentUrl} onClick={this.onClick}>
-                <img className="mx_MImageBody_thumbnail" src={thumbUrl} ref="image"
-                    alt={content.body}
-                    onError={this.onImageError}
-                    onLoad={this.onImageLoad}
-                    onMouseEnter={this.onImageEnter}
-                    onMouseLeave={this.onImageLeave} />
-            </a>
-        );
+    // Overidden by MStickerBody
+    getPlaceholder() {
+        // MImageBody doesn't show a placeholder whilst the image loads, (but it could do)
+        return null;
+    }
 
-        return (
-            <span className="mx_MImageBody" ref="body">
-                { thumbUrl && !this.state.imgError ? thumbnail : '' }
-                <MFileBody {...this.props} decryptedBlob={this.state.decryptedBlob} />
-            </span>
-      );
+    // Overidden by MStickerBody
+    getTooltip() {
+        return null;
+    }
+
+    // Overidden by MStickerBody
+    getFileBody() {
+        return <MFileBody {...this.props} decryptedBlob={this.state.decryptedBlob} />;
     }
 
     render() {
@@ -302,25 +332,6 @@ export default class extends React.Component {
             );
         }
 
-        if (content.file !== undefined && this.state.decryptedUrl === null) {
-            // Need to decrypt the attachment
-            // The attachment is decrypted in componentDidMount.
-            // For now add an img tag with a spinner.
-            return (
-                <span className="mx_MImageBody" ref="body">
-                    <div className="mx_MImageBody_thumbnail" ref="image" style={{
-                        "display": "flex",
-                        "alignItems": "center",
-                        "width": "100%",
-                    }}>
-                        <img src="img/spinner.gif" alt={content.body} width="32" height="32" style={{
-                            "margin": "auto",
-                        }} />
-                    </div>
-                </span>
-            );
-        }
-
         const contentUrl = this._getContentUrl();
         let thumbUrl;
         if (this._isGif() && SettingsStore.getValue("autoplayGifsAndVideos")) {
@@ -329,6 +340,12 @@ export default class extends React.Component {
           thumbUrl = this._getThumbUrl();
         }
 
-        return this._messageContent(contentUrl, thumbUrl, content);
+        const thumbnail = this._messageContent(contentUrl, thumbUrl, content);
+        const fileBody = this.getFileBody();
+
+        return <span className="mx_MImageBody" ref="body">
+            { thumbnail }
+            { fileBody }
+        </span>;
     }
 }