Merge pull request #2379 from tf/img-collapse

tf · web-flow · commit 7acbd2a1c74a · 2026-03-25T10:00:51.000+01:00
Fix inline image collapse when file is not ready
diff --git a/entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/placeholder-spec.js b/entry_types/scrolled/package/spec/contentElements/externalLinkList/frontend/ExternalLinkList/placeholder-spec.js
@@ -0,0 +1,30 @@
+import React from 'react';
+
+import {ExternalLinkList} from 'contentElements/externalLinkList/frontend/ExternalLinkList';
+
+import {renderInContentElement} from 'pageflow-scrolled/testHelpers';
+import '@testing-library/jest-dom/extend-expect'
+
+import styles from 'frontend/Placeholder.module.css';
+
+describe('ExternalLinkList placeholder', () => {
+  it('renders placeholder in editor', () => {
+    const {container} = renderInContentElement(
+      <ExternalLinkList configuration={{links: [{id: 1}]}}
+                        sectionProps={{}} />,
+      {editorState: {isEditable: true}}
+    );
+
+    expect(container.querySelector(`.${styles.placeholder}`)).not.toBeNull();
+  });
+
+  it('does not render placeholder in published entry', () => {
+    const {container} = renderInContentElement(
+      <ExternalLinkList configuration={{links: [{id: 1}]}}
+                        sectionProps={{}} />,
+      {editorState: {isEditable: false}}
+    );
+
+    expect(container.querySelector(`.${styles.placeholder}`)).toBeNull();
+  });
+});
diff --git a/entry_types/scrolled/package/spec/contentElements/inlineImage/InlineImage-spec.js b/entry_types/scrolled/package/spec/contentElements/inlineImage/InlineImage-spec.js
@@ -337,5 +337,23 @@ describe('InlineImage', () => {
       const contentElement = getContentElement();
       expect(contentElement.getFitViewportAspectRatio()).toEqual('0.75');
     });
+
+    it('uses default aspect ratio when file is not ready', () => {
+      const {getContentElement} = renderContentElement({
+        typeName: 'inlineImage',
+        configuration: {
+          id: 100
+        },
+        imageFiles: [{
+          permaId: 100,
+          isReady: false,
+          width: null,
+          height: null
+        }]
+      });
+
+      const contentElement = getContentElement();
+      expect(contentElement.getFitViewportAspectRatio()).toEqual('0.75');
+    });
   });
 });
diff --git a/entry_types/scrolled/package/spec/frontend/FilePlaceholderDecorator-spec.js b/entry_types/scrolled/package/spec/frontend/FilePlaceholderDecorator-spec.js
@@ -0,0 +1,32 @@
+import React from 'react';
+import {render} from '@testing-library/react';
+import '@testing-library/jest-dom/extend-expect';
+
+import {FilePlaceholder} from 'frontend/FilePlaceholderDecorator';
+import styles from 'frontend/Placeholder.module.css';
+
+describe('FilePlaceholder', () => {
+  it('renders placeholder when no file is given', () => {
+    const {container} = render(
+      <FilePlaceholder />
+    );
+
+    expect(container.querySelector(`.${styles.placeholder}`)).not.toBeNull();
+  });
+
+  it('renders placeholder when file is not ready', () => {
+    const {container} = render(
+      <FilePlaceholder file={{isReady: false}} />
+    );
+
+    expect(container.querySelector(`.${styles.placeholder}`)).not.toBeNull();
+  });
+
+  it('does not render placeholder when file is ready', () => {
+    const {container} = render(
+      <FilePlaceholder file={{isReady: true}} />
+    );
+
+    expect(container.querySelector(`.${styles.placeholder}`)).toBeNull();
+  });
+});
diff --git a/entry_types/scrolled/package/spec/frontend/FitViewport-spec.js b/entry_types/scrolled/package/spec/frontend/FitViewport-spec.js
@@ -61,35 +61,47 @@ describe('FitViewport', () => {
     expect(getOuter(container)).toHaveStyle('--fit-viewport-scale: 0.8');
   });
 
-  it('is not opaque by default', () => {
+  it('support covering full height', () => {
     const {container} = render(
-      <FitViewport aspectRatio={0.5}>
+      <FitViewport aspectRatio={0.5} fill>
         <FitViewport.Content />
       </FitViewport>
     );
 
-    expect(getOuter(container)).not.toHaveClass(styles.opaque);
+    expect(getOuter(container)).not.toHaveStyle('--fit-viewport-aspect-ratio: 0.5');
+    expect(container.querySelector(`.${fullscreenStyles.root}`)).not.toBeNull();
   });
 
-  it('can be made opaque', () => {
+  it('uses fallback aspect ratio when file is present but not ready', () => {
+    const file = {isReady: false};
     const {container} = render(
-      <FitViewport aspectRatio={0.5} opaque>
+      <FitViewport file={file} fallbackAspectRatio={0.75}>
         <FitViewport.Content />
       </FitViewport>
     );
 
-    expect(getOuter(container)).toHaveClass(styles.opaque);
+    expect(getOuter(container)).toHaveStyle('--fit-viewport-aspect-ratio: 0.75');
   });
 
-  it('support covering full height', () => {
+  it('uses file aspect ratio when file is ready even if fallback is given', () => {
+    const file = {width: 200, height: 100};
     const {container} = render(
-      <FitViewport aspectRatio={0.5} fill>
+      <FitViewport file={file} fallbackAspectRatio={0.75}>
         <FitViewport.Content />
       </FitViewport>
     );
 
-    expect(getOuter(container)).not.toHaveStyle('--fit-viewport-aspect-ratio: 0.5');
-    expect(container.querySelector(`.${fullscreenStyles.root}`)).not.toBeNull();
+    expect(getOuter(container)).toHaveStyle('--fit-viewport-aspect-ratio: 0.5');
+  });
+
+  it('uses fallback aspect ratio when no file is present', () => {
+    const {container} = render(
+      <FitViewport fallbackAspectRatio={0.75}>
+        <FitViewport.Content />
+      </FitViewport>
+    );
+
+    expect(getOuter(container)).toHaveStyle('--fit-viewport-aspect-ratio: 0.75');
   });
 
   function getOuter(container) {
diff --git a/entry_types/scrolled/package/src/contentElements/externalLinkList/frontend/ExternalLink.js b/entry_types/scrolled/package/src/contentElements/externalLinkList/frontend/ExternalLink.js
@@ -165,7 +165,8 @@ export function ExternalLink({id, configuration, contentElementId, ...props}) {
                        aspectRatio={props.thumbnailAspectRatio}
                        cropPosition={props.thumbnailCropPosition}
                        fit={props.thumbnailFit}
-                       load={props.loadImages}>
+                       load={props.loadImages}
+                       showPlaceholder={isEditable}>
               <InlineFileRights configuration={configuration}
                                 context="insideElement"
                                 position={props.textPosition === 'overlay' ? 'top': 'bottom'}
diff --git a/entry_types/scrolled/package/src/contentElements/externalLinkList/frontend/Thumbnail.js b/entry_types/scrolled/package/src/contentElements/externalLinkList/frontend/Thumbnail.js
@@ -1,11 +1,11 @@
 import React from 'react';
 import classNames from 'classnames';
 
-import {Image} from 'pageflow-scrolled/frontend';
+import {Image, FilePlaceholder} from 'pageflow-scrolled/frontend';
 
 import styles from './Thumbnail.module.css';
 
-export function Thumbnail({imageFile, aspectRatio, cropPosition, fit, load, children}) {
+export function Thumbnail({imageFile, aspectRatio, cropPosition, fit, load, showPlaceholder, children}) {
   imageFile = {
     ...imageFile,
     cropPosition
@@ -17,6 +17,7 @@ export function Thumbnail({imageFile, aspectRatio, cropPosition, fit, load, chil
     <div className={classNames(styles.thumbnail,
                                {[styles.cover]: fit === 'cover'})}
          style={{paddingTop: aspectRatioPadding}}>
+      {showPlaceholder && <FilePlaceholder file={imageFile} />}
       <Image imageFile={imageFile}
              load={load}
              preferSvg={true}
diff --git a/entry_types/scrolled/package/src/contentElements/hotspots/Hotspots.js b/entry_types/scrolled/package/src/contentElements/hotspots/Hotspots.js
@@ -5,6 +5,7 @@ import {
   ContentElementBox,
   Image,
   ContentElementFigure,
+  FilePlaceholder,
   FitViewport,
   FullscreenViewer,
   ToggleFullscreenCornerButton,
@@ -97,7 +98,7 @@ export function HotspotsImage({
   const {shouldLoad} = useContentElementLifecycle();
   const {isEditable, isSelected} = useContentElementEditorState();
 
-  const aspectRatio = imageFile ? `${imageFile.width} / ${imageFile.height}` : '3 / 4';
+  const aspectRatio = imageFile ? `${imageFile.width} / ${imageFile.height}` : '4 / 3';
 
   const [containerRect, containerRef] = useContentRect({
     enabled: shouldLoad
@@ -235,9 +236,8 @@ export function HotspotsImage({
            activeIndex={activeIndex}
            activateArea={activateArea}>
       <FitViewport file={imageFile}
-                   aspectRatio={imageFile ? undefined : 0.75}
-                   fill={configuration.position === 'backdrop'}
-                   opaque={!imageFile}>
+                   fallbackAspectRatio={0.75}
+                   fill={configuration.position === 'backdrop'}>
         <Composite activeIndex={activeIndex + 1}
                    loop={false}
                    onNavigate={index => activateArea(index - 1)}>
@@ -248,6 +248,7 @@ export function HotspotsImage({
               <FitViewport.Content>
                 <div className={styles.stack}
                      ref={containerRef}>
+                  <FilePlaceholder file={imageFile} />
                   {renderLetterboxBackground()}
                   <div className={styles.wrapper}
                        ref={panZoomRefs.wrapper}
diff --git a/entry_types/scrolled/package/src/contentElements/iframeEmbed/IframeEmbed.js b/entry_types/scrolled/package/src/contentElements/iframeEmbed/IframeEmbed.js
@@ -5,6 +5,7 @@ import {
   ContentElementBox,
   ContentElementFigure,
   FitViewport,
+  Placeholder,
   ThirdPartyOptIn,
   ThirdPartyOptOutInfo,
   useContentElementEditorState,
@@ -55,10 +56,10 @@ export function IframeEmbed({configuration}) {
   return (
     <div className={styles.wrapper}
          style={{pointerEvents: isEditable && !isSelected ? 'none' : undefined}}>
-      <FitViewport aspectRatio={configuration.autoResize ? null : aspectRatios[aspectRatio || 'wide']}
-                   opaque={utils.isBlank(configuration.source)}>
+      <FitViewport aspectRatio={configuration.autoResize ? null : aspectRatios[aspectRatio || 'wide']}>
         <ContentElementBox>
           <ContentElementFigure configuration={configuration}>
+            {utils.isBlank(configuration.source) && <Placeholder />}
             {renderSpanningWrapper(
               <ThirdPartyOptIn>
                 {shouldLoad &&
diff --git a/entry_types/scrolled/package/src/contentElements/imageGallery/ImageGallery.js b/entry_types/scrolled/package/src/contentElements/imageGallery/ImageGallery.js
@@ -7,6 +7,7 @@ import {
   useFileWithInlineRights,
   ContentElementBox,
   Figure,
+  FilePlaceholder,
   FitViewport,
   FullscreenViewer,
   Image,
@@ -250,16 +251,16 @@ function ItemImageWithCaption({item, imageFile, configuration, current, onClick,
 
   return (
     <FitViewport file={imageFile}
-                 aspectRatio={imageFile ? undefined : 0.75}
-                 scale={0.8}
-                 opaque={!imageFile}>
+                 fallbackAspectRatio={0.75}
+                 scale={0.8}>
       <ContentElementBox>
         <Figure caption={caption}
                 variant={configuration.captionVariant}
                 onCaptionChange={handleCaptionChange}
                 addCaptionButtonVisible={current && !item.placeholder}
                 addCaptionButtonPosition="inside">
           <FitViewport.Content>
+            <FilePlaceholder file={imageFile} />
             <div onClick={onClick}>
               <Image imageFile={imageFile} load={shouldLoad} />
             </div>
diff --git a/entry_types/scrolled/package/src/contentElements/inlineImage/InlineImage.js b/entry_types/scrolled/package/src/contentElements/inlineImage/InlineImage.js
@@ -4,6 +4,7 @@ import {
   ContentElementBox,
   Image,
   ContentElementFigure,
+  FilePlaceholder,
   FitViewport,
   contentElementWidths,
   useContentElementLifecycle,
@@ -90,8 +91,8 @@ function ImageWithCaption({
 
   return (
     <FitViewport file={imageFile}
-                 aspectRatio={aspectRatio || (imageFile ? undefined : 0.75)}
-                 opaque={!imageFile}>
+                 aspectRatio={aspectRatio}
+                 fallbackAspectRatio={0.75}>
       <ContentElementBox borderRadius={isCircleCrop ? 'none' : rounded}>
         <ContentElementFigure configuration={configuration}>
           <FitViewport.Content>
@@ -100,6 +101,7 @@ function ImageWithCaption({
               <ExpandableImage enabled={supportFullscreen && shouldLoad}
                                imageFile={imageFile}
                                contentElementId={contentElementId}>
+                <FilePlaceholder file={imageFile} />
                 <Image imageFile={imageFile}
                        load={shouldLoad}
                        structuredData={true}
diff --git a/entry_types/scrolled/package/src/contentElements/inlineVideo/InlineVideo.js b/entry_types/scrolled/package/src/contentElements/inlineVideo/InlineVideo.js
@@ -5,6 +5,7 @@ import {
   VideoPlayer,
   ContentElementBox,
   ContentElementFigure,
+  FilePlaceholder,
   MediaInteractionTracking,
   VideoPlayerControls,
   InlineFileRights,
@@ -123,13 +124,12 @@ function OrientationUnawareInlineVideo({
   return (
     <MediaInteractionTracking playerState={playerState} playerActions={playerActions}>
       <FitViewport file={videoFile}
-                   aspectRatio={videoFile ?
-                                undefined : fallbackAspectRatio}
-                   fill={configuration.position === 'backdrop'}
-                   opaque={!videoFile}>
+                   fallbackAspectRatio={fallbackAspectRatio}
+                   fill={configuration.position === 'backdrop'}>
         <ContentElementBox>
           <ContentElementFigure configuration={configuration}>
             <FitViewport.Content>
+              <FilePlaceholder file={videoFile} />
               <MutedIndicator visible={media.muted &&
                                        playerState.shouldPlay &&
                                        !configuration.keepMuted} />
diff --git a/entry_types/scrolled/package/src/frontend/FilePlaceholderDecorator.js b/entry_types/scrolled/package/src/frontend/FilePlaceholderDecorator.js
@@ -0,0 +1,10 @@
+import React from 'react';
+import {Placeholder} from './Placeholder';
+
+export function FilePlaceholder({file}) {
+  if (file?.isReady) {
+    return null;
+  }
+
+  return <Placeholder />;
+}
diff --git a/entry_types/scrolled/package/src/frontend/FitViewport.js b/entry_types/scrolled/package/src/frontend/FitViewport.js
@@ -1,5 +1,4 @@
 import React, {useContext} from 'react';
-import classNames from 'classnames';
 
 import styles from "./FitViewport.module.css";
 import Fullscreen from "./Fullscreen";
@@ -29,20 +28,21 @@ const AspectRatioContext = React.createContext();
  * @param {number} [props.aspectRatio] - Aspect ratio of div.
  * @param {Object} [props.file] - Use width/height of file to calculate aspect ratio.
  * @param {number} [props.scale] - Only take up fraction of the viewport height supplied as value between 0 and 1.
- * @param {Object} [props.opaque] - Render black background behind content.
  * @param {string} [props.fill] - Ignore aspect ration and fill viewport vertically.
  */
-export function FitViewport({file, aspectRatio, opaque, children, fill, scale}) {
-  if (!file && !aspectRatio) return children;
+export function FitViewport({file, aspectRatio, fallbackAspectRatio, children, fill, scale}) {
+  if (!file && !aspectRatio && !fallbackAspectRatio) return children;
 
   if (typeof aspectRatio === 'string') {
     aspectRatio = `var(--theme-aspect-ratio-${aspectRatio})`;
   }
 
-  aspectRatio = fill ? 'fill' : aspectRatio || (file.height / file.width);
+  aspectRatio = fill ? 'fill' :
+                aspectRatio ||
+                (file?.width ? file.height / file.width : fallbackAspectRatio);
 
   return (
-    <div className={classNames(styles.container, {[styles.opaque]: opaque})}
+    <div className={styles.container}
          style={{'--fit-viewport-aspect-ratio': fill ? undefined : aspectRatio,
                  '--fit-viewport-scale': scale}}>
       <AspectRatioContext.Provider value={aspectRatio}>
diff --git a/entry_types/scrolled/package/src/frontend/FitViewport.module.css b/entry_types/scrolled/package/src/frontend/FitViewport.module.css
@@ -23,11 +23,6 @@
   bottom: 0;
 }
 
-.opaque .inner {
-  background-color: #000;
-  color: #fff;
-}
-
 @media print {
   .container {
     page-break-inside: avoid;
diff --git a/entry_types/scrolled/package/src/frontend/Placeholder.js b/entry_types/scrolled/package/src/frontend/Placeholder.js
@@ -0,0 +1,7 @@
+import React from 'react';
+import {withInlineEditingAlternative} from './inlineEditing';
+import styles from './Placeholder.module.css';
+
+export const Placeholder = withInlineEditingAlternative('Placeholder', function Placeholder() {
+  return <div className={styles.placeholder} />;
+});
diff --git a/entry_types/scrolled/package/src/frontend/Placeholder.module.css b/entry_types/scrolled/package/src/frontend/Placeholder.module.css
@@ -0,0 +1,5 @@
+.placeholder {
+  position: absolute;
+  inset: 0;
+  background: #000;
+}
diff --git a/entry_types/scrolled/package/src/frontend/index.js b/entry_types/scrolled/package/src/frontend/index.js
@@ -110,6 +110,8 @@ export {EditableText} from './EditableText';
 export {EditableInlineText} from './EditableInlineText';
 export {EditableLink} from './EditableLink';
 export {ActionButton} from './ActionButton';
+export {FilePlaceholder} from './FilePlaceholderDecorator';
+export {Placeholder} from './Placeholder';
 export {LinkTooltipProvider} from './LinkTooltipProvider';
 export {PhonePlatformContext} from './PhonePlatformContext';
 export {
diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/Placeholder.js b/entry_types/scrolled/package/src/frontend/inlineEditing/Placeholder.js
diff --git a/entry_types/scrolled/package/src/frontend/inlineEditing/components.js b/entry_types/scrolled/package/src/frontend/inlineEditing/components.js
