From 82c57d71816913b181883983a3468a10c1ef4ac1 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Tue, 4 Jun 2024 18:10:30 +0100 Subject: [PATCH 01/35] use hard coded constant so all trails are required to be portrait --- fronts-client/src/components/card/Card.tsx | 10 ++++++++-- .../src/components/form/ArticleMetaForm.tsx | 16 +++++++++++++--- .../src/components/inputs/InputImage.tsx | 9 ++++++++- fronts-client/src/constants/image.ts | 8 +++++++- 4 files changed, 36 insertions(+), 7 deletions(-) diff --git a/fronts-client/src/components/card/Card.tsx b/fronts-client/src/components/card/Card.tsx index a7ee6f1958d..c9ef51604fb 100644 --- a/fronts-client/src/components/card/Card.tsx +++ b/fronts-client/src/components/card/Card.tsx @@ -21,9 +21,10 @@ import { ValidationResponse, } from 'util/validateImageSrc'; import { - cardImageCriteria, + landScapeCardImageCriteria, editionsCardImageCriteria, DRAG_DATA_CARD_IMAGE_OVERRIDE, + portraitCardImageCriteria, } from 'constants/image'; import Sublinks from '../FrontsEdit/CollectionComponents/Sublinks'; import { @@ -49,6 +50,9 @@ import { ChefMetaForm } from '../form/ChefMetaForm'; export const createCardId = (id: string) => `collection-item-${id}`; +// TO DO - replace this constant with logic based on container type +const FORCE_ALL_CARDS_TO_USE_PORTRAIT_TRAILS = true as boolean; + const CardContainer = styled('div')<{ pillarId: string | undefined; isLive?: boolean; @@ -346,7 +350,9 @@ class Card extends React.Component { const isEditionsMode = this.props.editMode === 'editions'; const imageCriteria = isEditionsMode ? editionsCardImageCriteria - : cardImageCriteria; + : FORCE_ALL_CARDS_TO_USE_PORTRAIT_TRAILS + ? portraitCardImageCriteria + : landScapeCardImageCriteria; // Our drag contains Grid data validateImageEvent(e, this.props.frontId, imageCriteria) diff --git a/fronts-client/src/components/form/ArticleMetaForm.tsx b/fronts-client/src/components/form/ArticleMetaForm.tsx index 9844dc10241..64ba296a103 100644 --- a/fronts-client/src/components/form/ArticleMetaForm.tsx +++ b/fronts-client/src/components/form/ArticleMetaForm.tsx @@ -42,10 +42,11 @@ import { import { CapiFields } from 'util/form'; import { Dispatch } from 'types/Store'; import { - cardImageCriteria, + landScapeCardImageCriteria, editionsCardImageCriteria, editionsMobileCardImageCriteria, editionsTabletCardImageCriteria, + portraitCardImageCriteria, } from 'constants/image'; import { selectors as collectionSelectors } from 'bundles/collectionsBundle'; import { getContributorImage } from 'util/CAPIUtils'; @@ -64,6 +65,9 @@ import { FormContent } from 'components/form/FormContent'; import { TextOptionsInputGroup } from 'components/form/TextOptionsInputGroup'; import { FormButtonContainer } from 'components/form/FormButtonContainer'; +// TO DO - replace this constant with logic based on container type +const FORCE_ALL_CARDS_TO_USE_PORTRAIT_TRAILS = true as boolean; + interface ComponentProps extends ContainerProps { articleExists: boolean; collectionId: string | null; @@ -284,7 +288,8 @@ const RenderSlideshow = ({ name={name} component={InputImage} small - criteria={cardImageCriteria} + // TO DO - will this always be landscape? + criteria={landScapeCardImageCriteria} frontId={frontId} isSelected={index === slideshowIndex} isInvalid={isInvalidCaptionLength(index)} @@ -682,7 +687,9 @@ class FormComponent extends React.Component { criteria={ isEditionsMode ? editionsCardImageCriteria - : cardImageCriteria + : FORCE_ALL_CARDS_TO_USE_PORTRAIT_TRAILS + ? portraitCardImageCriteria + : landScapeCardImageCriteria } frontId={frontId} defaultImageUrl={ @@ -694,6 +701,9 @@ class FormComponent extends React.Component { message={ imageCutoutReplace ? 'Add cutout' : 'Replace image' } + requirePortraitTrails={ + FORCE_ALL_CARDS_TO_USE_PORTRAIT_TRAILS + } hasVideo={hasMainVideo} onChange={this.handleImageChange} /> diff --git a/fronts-client/src/components/inputs/InputImage.tsx b/fronts-client/src/components/inputs/InputImage.tsx index a1d2fb931cc..c9fb37fcdcd 100644 --- a/fronts-client/src/components/inputs/InputImage.tsx +++ b/fronts-client/src/components/inputs/InputImage.tsx @@ -228,6 +228,7 @@ export interface InputImageContainerProps { replaceImage: boolean; isSelected?: boolean; isInvalid?: boolean; + requirePortraitTrails?: boolean; } type ComponentProps = InputImageContainerProps & @@ -268,6 +269,7 @@ class InputImage extends React.Component { disabled, isSelected, isInvalid, + requirePortraitTrails = false, } = this.props; if (!gridUrl) { @@ -279,7 +281,11 @@ class InputImage extends React.Component { } const gridSearchUrl = - editMode === 'editions' ? `${gridUrl}` : `${gridUrl}?cropType=landscape`; + editMode === 'editions' + ? `${gridUrl}` + : requirePortraitTrails + ? `${gridUrl}?cropType=portrait` + : `${gridUrl}?cropType=landscape`; const hasImage = !useDefault && !!input.value && !!input.value.thumb; const imageUrl = !useDefault && input.value && input.value.thumb @@ -305,6 +311,7 @@ class InputImage extends React.Component { onDragIntentStart={() => this.setState({ isDragging: true })} onDragIntentEnd={() => this.setState({ isDragging: false })} > + {/* TO DO - need to update this component to allow for portrait images */} Date: Wed, 5 Jun 2024 13:58:57 +0100 Subject: [PATCH 02/35] allow grid to select either image type, add state for the dimension of the selected replacement image, use to style the ImageContainer --- .../src/components/form/ArticleMetaForm.tsx | 4 +- .../src/components/inputs/InputImage.tsx | 93 +++++++++++++++---- 2 files changed, 77 insertions(+), 20 deletions(-) diff --git a/fronts-client/src/components/form/ArticleMetaForm.tsx b/fronts-client/src/components/form/ArticleMetaForm.tsx index 64ba296a103..05dfec660c4 100644 --- a/fronts-client/src/components/form/ArticleMetaForm.tsx +++ b/fronts-client/src/components/form/ArticleMetaForm.tsx @@ -701,9 +701,7 @@ class FormComponent extends React.Component { message={ imageCutoutReplace ? 'Add cutout' : 'Replace image' } - requirePortraitTrails={ - FORCE_ALL_CARDS_TO_USE_PORTRAIT_TRAILS - } + allowPortraitTrails={FORCE_ALL_CARDS_TO_USE_PORTRAIT_TRAILS} hasVideo={hasMainVideo} onChange={this.handleImageChange} /> diff --git a/fronts-client/src/components/inputs/InputImage.tsx b/fronts-client/src/components/inputs/InputImage.tsx index c9fb37fcdcd..4082b64514d 100644 --- a/fronts-client/src/components/inputs/InputImage.tsx +++ b/fronts-client/src/components/inputs/InputImage.tsx @@ -35,6 +35,7 @@ import { error } from '../../styleConstants'; const ImageContainer = styled.div<{ small?: boolean; + portrait?: boolean; }>` display: flex; flex-direction: column; @@ -47,6 +48,13 @@ const ImageContainer = styled.div<{ padding: 40%;`} height: ${(props) => (props.small ? '0' : '115px')}; transition: background-color 0.15s; + + ${({ portrait, small }) => + portrait && + ` + width: ${small ? 50 : 200}px; + height: ${small ? 62 : 250}px; + `} `; const AddImageButton = styled(ButtonDefault)<{ small?: boolean }>` @@ -228,7 +236,7 @@ export interface InputImageContainerProps { replaceImage: boolean; isSelected?: boolean; isInvalid?: boolean; - requirePortraitTrails?: boolean; + allowPortraitTrails?: boolean; } type ComponentProps = InputImageContainerProps & @@ -238,6 +246,10 @@ interface ComponentState { isDragging: boolean; modalOpen: boolean; imageSrc: string; + imageDims?: { + width: number; + height: number; + }; confirmDelete: boolean; cancelDeleteTimeout: undefined | (() => void); } @@ -246,16 +258,37 @@ const dragImage = new Image(); dragImage.src = imageDragIcon; class InputImage extends React.Component { - public state = { - isDragging: false, - modalOpen: false, - imageSrc: '', - confirmDelete: false, - cancelDeleteTimeout: undefined, - } as ComponentState; - private inputRef = React.createRef(); + public constructor(props: ComponentProps) { + super(props); + + const { value } = props.input; + const valueRecord = + value && typeof value === 'object' + ? (value as Record) + : undefined; + + const { src, height, width } = valueRecord ?? {}; + const imageSrc = typeof src === 'string' ? src : ''; + const imageDims = + typeof height === 'number' && typeof width === 'number' + ? { + height, + width, + } + : undefined; + + this.state = { + isDragging: false, + modalOpen: false, + imageSrc, + imageDims, + confirmDelete: false, + cancelDeleteTimeout: undefined, + } as ComponentState; + } + public render() { const { small = false, @@ -269,8 +302,9 @@ class InputImage extends React.Component { disabled, isSelected, isInvalid, - requirePortraitTrails = false, + allowPortraitTrails = false, } = this.props; + const { imageDims } = this.state; if (!gridUrl) { return ( @@ -283,14 +317,21 @@ class InputImage extends React.Component { const gridSearchUrl = editMode === 'editions' ? `${gridUrl}` - : requirePortraitTrails - ? `${gridUrl}?cropType=portrait` + : allowPortraitTrails + ? `${gridUrl}?cropType=landscape,portrait` : `${gridUrl}?cropType=landscape`; const hasImage = !useDefault && !!input.value && !!input.value.thumb; const imageUrl = !useDefault && input.value && input.value.thumb ? input.value.thumb : defaultImageUrl; + + const imageContainerShouldBePortrait = !!( + !useDefault && + imageDims && + imageDims.height > imageDims.width + ); + return ( { onDragIntentStart={() => this.setState({ isDragging: true })} onDragIntentEnd={() => this.setState({ isDragging: false })} > - {/* TO DO - need to update this component to allow for portrait images */} - + { private validateAndGetImage = () => { events.imageAdded(this.props.frontId, 'paste'); - validateImageSrc( this.state.imageSrc, this.props.frontId, this.props.criteria ) .then((mediaItem) => { + this.setState({ + imageDims: { + height: mediaItem.height, + width: mediaItem.width, + }, + }); this.props.input.onChange(mediaItem); }) .catch((err) => { @@ -469,10 +517,15 @@ class InputImage extends React.Component { // tslint:disable-next-line no-console console.log('@todo:handle error', err); }); - this.setState({ imageSrc: '' }); + this.setState({ imageSrc: '', imageDims: undefined }); }; - private clearField = () => this.props.input.onChange(null); + private clearField = () => { + this.setState({ + imageDims: undefined, + }); + return this.props.input.onChange(null); + }; private validMessage(data: GridData) { return data && data.crop && data.crop.data && data.image && data.image.data; @@ -509,6 +562,12 @@ class InputImage extends React.Component { ) .then((mediaItem) => { events.imageAdded(this.props.frontId, 'click to modal'); + this.setState({ + imageDims: { + width: mediaItem.width, + height: mediaItem.height, + }, + }); this.props.input.onChange(mediaItem); }) .catch((err) => { From f81efffbad3064a760f99d8b7f232891ea5d161e Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Wed, 5 Jun 2024 14:14:57 +0100 Subject: [PATCH 03/35] bug fix? aspect ratio test should use absolute difference --- fronts-client/src/util/validateImageSrc.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fronts-client/src/util/validateImageSrc.ts b/fronts-client/src/util/validateImageSrc.ts index f08973e6da2..0a42b72f0ff 100644 --- a/fronts-client/src/util/validateImageSrc.ts +++ b/fronts-client/src/util/validateImageSrc.ts @@ -140,7 +140,7 @@ function validateActualImage(image: ImageDescription, frontId?: string) { return reject( new Error(`Images cannot be less than ${minWidth} pixels wide`) ); - } else if (criteriaRatio && criteriaRatio - ratio > 0.01) { + } else if (criteriaRatio && Math.abs(criteriaRatio - ratio) > 0.01) { return reject( new Error( `Images must have a ${widthAspectRatio || ''}:${ From cdfd2d6a67f24fddb62e38a1b91cf29afee38fe4 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Wed, 5 Jun 2024 14:41:07 +0100 Subject: [PATCH 04/35] use the criteria to set the gird url, not a separate prop --- .../src/components/form/ArticleMetaForm.tsx | 1 - .../src/components/inputs/InputImage.tsx | 29 ++++++++++++++----- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/fronts-client/src/components/form/ArticleMetaForm.tsx b/fronts-client/src/components/form/ArticleMetaForm.tsx index 05dfec660c4..71863a5ceca 100644 --- a/fronts-client/src/components/form/ArticleMetaForm.tsx +++ b/fronts-client/src/components/form/ArticleMetaForm.tsx @@ -701,7 +701,6 @@ class FormComponent extends React.Component { message={ imageCutoutReplace ? 'Add cutout' : 'Replace image' } - allowPortraitTrails={FORCE_ALL_CARDS_TO_USE_PORTRAIT_TRAILS} hasVideo={hasMainVideo} onChange={this.handleImageChange} /> diff --git a/fronts-client/src/components/inputs/InputImage.tsx b/fronts-client/src/components/inputs/InputImage.tsx index 4082b64514d..597463e5d1a 100644 --- a/fronts-client/src/components/inputs/InputImage.tsx +++ b/fronts-client/src/components/inputs/InputImage.tsx @@ -26,7 +26,10 @@ import { WarningIcon, } from '../icons/Icons'; import imageDragIcon from 'images/icons/image-drag-icon.svg'; -import { DRAG_DATA_GRID_IMAGE_URL } from 'constants/image'; +import { + DRAG_DATA_GRID_IMAGE_URL, + portraitCardImageCriteria, +} from 'constants/image'; import ImageDragIntentIndicator from 'components/image/ImageDragIntentIndicator'; import { EditMode } from 'types/EditMode'; import { selectEditMode } from '../../selectors/pathSelectors'; @@ -236,7 +239,6 @@ export interface InputImageContainerProps { replaceImage: boolean; isSelected?: boolean; isInvalid?: boolean; - allowPortraitTrails?: boolean; } type ComponentProps = InputImageContainerProps & @@ -302,7 +304,6 @@ class InputImage extends React.Component { disabled, isSelected, isInvalid, - allowPortraitTrails = false, } = this.props; const { imageDims } = this.state; @@ -315,11 +316,7 @@ class InputImage extends React.Component { } const gridSearchUrl = - editMode === 'editions' - ? `${gridUrl}` - : allowPortraitTrails - ? `${gridUrl}?cropType=landscape,portrait` - : `${gridUrl}?cropType=landscape`; + editMode === 'editions' ? `${gridUrl}` : this.criteriaToGridUrl(); const hasImage = !useDefault && !!input.value && !!input.value.thumb; const imageUrl = !useDefault && input.value && input.value.thumb @@ -586,6 +583,22 @@ class InputImage extends React.Component { this.setState({ modalOpen: true }); window.addEventListener('message', this.onMessage, false); }; + + private criteriaToGridUrl = (): string => { + const { criteria, gridUrl } = this.props; + + if (!criteria) { + return `${gridUrl}?cropType=portrait,landscape`; + } + + const usingPortrait = + portraitCardImageCriteria.widthAspectRatio == criteria.widthAspectRatio && + portraitCardImageCriteria.heightAspectRatio == criteria.heightAspectRatio; + + return usingPortrait + ? `${gridUrl}?cropType=portrait` + : `${gridUrl}?cropType=landscape`; + }; } const mapStateToProps = (state: State) => { From 32be7d898685142d396bdc8f42fef280f98df6e6 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Wed, 5 Jun 2024 15:56:30 +0100 Subject: [PATCH 05/35] export the default trail image criteria from constants --- fronts-client/src/components/card/Card.tsx | 8 ++------ fronts-client/src/components/form/ArticleMetaForm.tsx | 11 +++-------- fronts-client/src/constants/image.ts | 2 ++ 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/fronts-client/src/components/card/Card.tsx b/fronts-client/src/components/card/Card.tsx index c9ef51604fb..5d94fe1aab6 100644 --- a/fronts-client/src/components/card/Card.tsx +++ b/fronts-client/src/components/card/Card.tsx @@ -25,6 +25,7 @@ import { editionsCardImageCriteria, DRAG_DATA_CARD_IMAGE_OVERRIDE, portraitCardImageCriteria, + defaultCardTrailImageCriteria, } from 'constants/image'; import Sublinks from '../FrontsEdit/CollectionComponents/Sublinks'; import { @@ -50,9 +51,6 @@ import { ChefMetaForm } from '../form/ChefMetaForm'; export const createCardId = (id: string) => `collection-item-${id}`; -// TO DO - replace this constant with logic based on container type -const FORCE_ALL_CARDS_TO_USE_PORTRAIT_TRAILS = true as boolean; - const CardContainer = styled('div')<{ pillarId: string | undefined; isLive?: boolean; @@ -350,9 +348,7 @@ class Card extends React.Component { const isEditionsMode = this.props.editMode === 'editions'; const imageCriteria = isEditionsMode ? editionsCardImageCriteria - : FORCE_ALL_CARDS_TO_USE_PORTRAIT_TRAILS - ? portraitCardImageCriteria - : landScapeCardImageCriteria; + : defaultCardTrailImageCriteria; // Our drag contains Grid data validateImageEvent(e, this.props.frontId, imageCriteria) diff --git a/fronts-client/src/components/form/ArticleMetaForm.tsx b/fronts-client/src/components/form/ArticleMetaForm.tsx index 71863a5ceca..536c093c921 100644 --- a/fronts-client/src/components/form/ArticleMetaForm.tsx +++ b/fronts-client/src/components/form/ArticleMetaForm.tsx @@ -46,7 +46,7 @@ import { editionsCardImageCriteria, editionsMobileCardImageCriteria, editionsTabletCardImageCriteria, - portraitCardImageCriteria, + defaultCardTrailImageCriteria, } from 'constants/image'; import { selectors as collectionSelectors } from 'bundles/collectionsBundle'; import { getContributorImage } from 'util/CAPIUtils'; @@ -65,9 +65,6 @@ import { FormContent } from 'components/form/FormContent'; import { TextOptionsInputGroup } from 'components/form/TextOptionsInputGroup'; import { FormButtonContainer } from 'components/form/FormButtonContainer'; -// TO DO - replace this constant with logic based on container type -const FORCE_ALL_CARDS_TO_USE_PORTRAIT_TRAILS = true as boolean; - interface ComponentProps extends ContainerProps { articleExists: boolean; collectionId: string | null; @@ -288,7 +285,7 @@ const RenderSlideshow = ({ name={name} component={InputImage} small - // TO DO - will this always be landscape? + // TO DO - will slideshows always be landscape? criteria={landScapeCardImageCriteria} frontId={frontId} isSelected={index === slideshowIndex} @@ -687,9 +684,7 @@ class FormComponent extends React.Component { criteria={ isEditionsMode ? editionsCardImageCriteria - : FORCE_ALL_CARDS_TO_USE_PORTRAIT_TRAILS - ? portraitCardImageCriteria - : landScapeCardImageCriteria + : defaultCardTrailImageCriteria } frontId={frontId} defaultImageUrl={ diff --git a/fronts-client/src/constants/image.ts b/fronts-client/src/constants/image.ts index 2452f25ef93..e3dfb2d7b46 100644 --- a/fronts-client/src/constants/image.ts +++ b/fronts-client/src/constants/image.ts @@ -10,6 +10,8 @@ export const portraitCardImageCriteria = { heightAspectRatio: 5, }; +export const defaultCardTrailImageCriteria = landScapeCardImageCriteria; + export const editionsCardImageCriteria = { minWidth: 400, }; From 3a1cb1b257e8b7e473188554f29a1a3c343debfa Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Wed, 5 Jun 2024 16:04:07 +0100 Subject: [PATCH 06/35] remove unused imports --- fronts-client/src/components/card/Card.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/fronts-client/src/components/card/Card.tsx b/fronts-client/src/components/card/Card.tsx index 5d94fe1aab6..6cab6b3db14 100644 --- a/fronts-client/src/components/card/Card.tsx +++ b/fronts-client/src/components/card/Card.tsx @@ -21,10 +21,8 @@ import { ValidationResponse, } from 'util/validateImageSrc'; import { - landScapeCardImageCriteria, editionsCardImageCriteria, DRAG_DATA_CARD_IMAGE_OVERRIDE, - portraitCardImageCriteria, defaultCardTrailImageCriteria, } from 'constants/image'; import Sublinks from '../FrontsEdit/CollectionComponents/Sublinks'; From fa508704bee6e38f457a630a2df648d4cd65fd36 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Wed, 5 Jun 2024 16:33:48 +0100 Subject: [PATCH 07/35] shorted var name, comments about use of constant criteria --- fronts-client/src/components/inputs/InputImage.tsx | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/fronts-client/src/components/inputs/InputImage.tsx b/fronts-client/src/components/inputs/InputImage.tsx index 597463e5d1a..9a19f49438d 100644 --- a/fronts-client/src/components/inputs/InputImage.tsx +++ b/fronts-client/src/components/inputs/InputImage.tsx @@ -36,6 +36,9 @@ import { selectEditMode } from '../../selectors/pathSelectors'; import CircularIconContainer from '../icons/CircularIconContainer'; import { error } from '../../styleConstants'; +// assuming any portrait image (ie height>width) +// is in the 4:5 ratio for purposes of styling +// the image container const ImageContainer = styled.div<{ small?: boolean; portrait?: boolean; @@ -323,7 +326,7 @@ class InputImage extends React.Component { ? input.value.thumb : defaultImageUrl; - const imageContainerShouldBePortrait = !!( + const portraitImage = !!( !useDefault && imageDims && imageDims.height > imageDims.width @@ -349,10 +352,7 @@ class InputImage extends React.Component { onDragIntentStart={() => this.setState({ isDragging: true })} onDragIntentEnd={() => this.setState({ isDragging: false })} > - + { return `${gridUrl}?cropType=portrait,landscape`; } + // assumes the only criteria that will be passed as props the defined + // constants for portrait(4:5) and landscape (5:3) const usingPortrait = portraitCardImageCriteria.widthAspectRatio == criteria.widthAspectRatio && portraitCardImageCriteria.heightAspectRatio == criteria.heightAspectRatio; From 3663f8c24a23b2adfa01955d8ca7618204ded2fd Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Thu, 6 Jun 2024 15:53:40 +0100 Subject: [PATCH 08/35] add selectCollectionType selector --- fronts-client/src/selectors/frontsSelectors.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/fronts-client/src/selectors/frontsSelectors.ts b/fronts-client/src/selectors/frontsSelectors.ts index 6a6c0c7f56b..8094ac263de 100644 --- a/fronts-client/src/selectors/frontsSelectors.ts +++ b/fronts-client/src/selectors/frontsSelectors.ts @@ -147,6 +147,16 @@ const selectCollectionDisplayName = ( return !!collection ? collection.displayName : ''; }; +const selectCollectionType = ( + state: State, + collectionId: string +): string | undefined => { + const collection = selectCollection(state, { + collectionId, + }); + return collection?.type; +}; + const selectFrontsIds = createSelector([selectFronts], (fronts): string[] => { if (!fronts) { return []; @@ -363,6 +373,7 @@ export { selectCollectionHasPrefill, selectCollectionIsHidden, selectCollectionDisplayName, + selectCollectionType, selectFrontsConfig, selectCollectionConfigs, selectFrontsIds, From 7ff52d78b428f62b8dc2bf3a0e36bce011f65f8b Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Thu, 6 Jun 2024 16:40:19 +0100 Subject: [PATCH 09/35] provide the collection type to the InputImage --- fronts-client/src/components/form/ArticleMetaForm.tsx | 9 ++++++++- fronts-client/src/components/inputs/InputImage.tsx | 3 +++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/fronts-client/src/components/form/ArticleMetaForm.tsx b/fronts-client/src/components/form/ArticleMetaForm.tsx index 9844dc10241..ae4b824c9a1 100644 --- a/fronts-client/src/components/form/ArticleMetaForm.tsx +++ b/fronts-client/src/components/form/ArticleMetaForm.tsx @@ -63,6 +63,7 @@ import { FormContainer } from 'components/form/FormContainer'; import { FormContent } from 'components/form/FormContent'; import { TextOptionsInputGroup } from 'components/form/TextOptionsInputGroup'; import { FormButtonContainer } from 'components/form/FormButtonContainer'; +import { selectCollectionType } from 'selectors/frontsSelectors'; interface ComponentProps extends ContainerProps { articleExists: boolean; @@ -80,6 +81,7 @@ interface ComponentProps extends ContainerProps { coverCardTabletImage?: ImageData; size?: string; isEmailFronts?: boolean; + collectionType?: string; } type Props = ComponentProps & @@ -696,6 +698,7 @@ class FormComponent extends React.Component { } hasVideo={hasMainVideo} onChange={this.handleImageChange} + collectionType={this.props.collectionType} /> @@ -1012,11 +1015,12 @@ const createMapStateToProps = () => { } const isEmailFronts = selectV2SubPath(state) === '/email'; + const collectionId = (parentCollection && parentCollection.id) || null; return { articleExists: !!article, hasMainVideo: !!article && !!article.hasMainVideo, - collectionId: (parentCollection && parentCollection.id) || null, + collectionId, getLastUpdatedBy, snapType: article && article.snapType, initialValues: getInitialValuesForCardForm(article), @@ -1043,6 +1047,9 @@ const createMapStateToProps = () => { coverCardTabletImage: valueSelector(state, 'coverCardTabletImage'), pickedKicker: !!article ? article.pickedKicker : undefined, isEmailFronts, + collectionType: collectionId + ? selectCollectionType(state, collectionId) + : undefined, }; }; }; diff --git a/fronts-client/src/components/inputs/InputImage.tsx b/fronts-client/src/components/inputs/InputImage.tsx index a1d2fb931cc..397e8874ce4 100644 --- a/fronts-client/src/components/inputs/InputImage.tsx +++ b/fronts-client/src/components/inputs/InputImage.tsx @@ -228,6 +228,7 @@ export interface InputImageContainerProps { replaceImage: boolean; isSelected?: boolean; isInvalid?: boolean; + collectionType?: string; } type ComponentProps = InputImageContainerProps & @@ -268,6 +269,7 @@ class InputImage extends React.Component { disabled, isSelected, isInvalid, + collectionType, } = this.props; if (!gridUrl) { @@ -293,6 +295,7 @@ class InputImage extends React.Component { isInvalid={isInvalid} confirmDelete={this.state.confirmDelete} > +

collectionType: {collectionType}

Date: Thu, 6 Jun 2024 17:45:52 +0100 Subject: [PATCH 10/35] add feature switch for whether to allow portrait crops --- app/model/FeatureSwitches.scala | 24 ++++++++++++++----- .../src/components/Features/FeaturesForm.tsx | 8 ++++++- .../src/components/form/ArticleMetaForm.tsx | 11 +++++++++ fronts-client/src/types/Features.ts | 1 + 4 files changed, 37 insertions(+), 7 deletions(-) diff --git a/app/model/FeatureSwitches.scala b/app/model/FeatureSwitches.scala index 0383ee28ca6..ad059d081b9 100644 --- a/app/model/FeatureSwitches.scala +++ b/app/model/FeatureSwitches.scala @@ -9,35 +9,47 @@ object FeatureSwitch { case class FeatureSwitch( key: String, title: String, - enabled: Boolean + enabled: Boolean, + hideOnProd: Boolean ) object ObscureFeed extends FeatureSwitch( key = "obscure-feed", title = "Obscure the feed -- it's distracting for developers!", - enabled = false + enabled = false, + hideOnProd = false ) object PageViewDataVisualisation extends FeatureSwitch( key = "page-view-data-visualisation", title = "Show page view data visualisation (aka spark lines)", - enabled = true + enabled = true, + hideOnProd = false ) object ShowFirefoxPrompt extends FeatureSwitch( key = "show-firefox-prompt", title = "Show the prompt to use Firefox if applicable", - enabled = true + enabled = false, + hideOnProd = false ) object TenImageSlideshows extends FeatureSwitch( key = "ten-image-slideshows", title = "Allow slideshows to contain 10 images rather than 5", - enabled = false + enabled = false, + hideOnProd = false +) + +object UsePortraitCropsForSomeCollectionTypes extends FeatureSwitch( + key = "support-portrait-crops", + title = "Use portrait crops for the experimental big card containers", + enabled = false, + hideOnProd = true ) object FeatureSwitches { - val all: List[FeatureSwitch] = List(ObscureFeed, PageViewDataVisualisation, ShowFirefoxPrompt, TenImageSlideshows) + val all: List[FeatureSwitch] = List(ObscureFeed, PageViewDataVisualisation, ShowFirefoxPrompt, TenImageSlideshows, UsePortraitCropsForSomeCollectionTypes) def updateFeatureSwitchesForUser(userDataSwitches: Option[List[FeatureSwitch]], switch: FeatureSwitch): List[FeatureSwitch] = { val newSwitches = userDataSwitches match { diff --git a/fronts-client/src/components/Features/FeaturesForm.tsx b/fronts-client/src/components/Features/FeaturesForm.tsx index ba5e54f0896..144c2dabf81 100644 --- a/fronts-client/src/components/Features/FeaturesForm.tsx +++ b/fronts-client/src/components/Features/FeaturesForm.tsx @@ -5,6 +5,7 @@ import type { State } from 'types/State'; import { selectAllFeatures } from 'selectors/featureSwitchesSelectors'; import { FeatureSwitch } from 'types/Features'; import { Dispatch } from 'types/Store'; +import pageConfig from 'util/extractConfigFromPage'; import { actionSetFeatureValue } from 'actions/FeatureSwitches'; import { saveFeatureSwitch } from 'services/userDataApi'; @@ -13,12 +14,17 @@ interface Props { setFeatureValue: (featureSwitch: FeatureSwitch) => void; } +const STAGE = pageConfig.env; +const filterSwitchesByStage = (featureSwitch: FeatureSwitch): boolean => + STAGE === 'code' || !featureSwitch.hideOnProd; + class FeaturesForm extends React.Component { public render() { const { featureSwitches } = this.props; + console.log(featureSwitches); return (
- {featureSwitches.map((featureSwitch) => ( + {featureSwitches.filter(filterSwitchesByStage).map((featureSwitch) => ( feature.key === 'support-portrait-crops' +); + +const SUPPORT_PORTRAIT_CROPS = supportPortraitCropsSwitch + ? supportPortraitCropsSwitch.enabled + : false; + +console.log({ SUPPORT_PORTRAIT_CROPS }); + interface ComponentProps extends ContainerProps { articleExists: boolean; collectionId: string | null; diff --git a/fronts-client/src/types/Features.ts b/fronts-client/src/types/Features.ts index a47fe7bb0ee..c342935ed60 100644 --- a/fronts-client/src/types/Features.ts +++ b/fronts-client/src/types/Features.ts @@ -2,4 +2,5 @@ export interface FeatureSwitch { key: string; title: string; enabled: boolean; + hideOnProd: boolean; } From 3f670a0c8e0e6d18a2046484f953a20351233b47 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Thu, 6 Jun 2024 17:58:36 +0100 Subject: [PATCH 11/35] if the feature switch is enabled, set criteria based on collection type --- .../src/components/form/ArticleMetaForm.tsx | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/fronts-client/src/components/form/ArticleMetaForm.tsx b/fronts-client/src/components/form/ArticleMetaForm.tsx index 7bdfe6d060c..ce257134e75 100644 --- a/fronts-client/src/components/form/ArticleMetaForm.tsx +++ b/fronts-client/src/components/form/ArticleMetaForm.tsx @@ -48,6 +48,7 @@ import { editionsMobileCardImageCriteria, editionsTabletCardImageCriteria, defaultCardTrailImageCriteria, + portraitCardImageCriteria, } from 'constants/image'; import { selectors as collectionSelectors } from 'bundles/collectionsBundle'; import { getContributorImage } from 'util/CAPIUtils'; @@ -66,6 +67,7 @@ import { FormContent } from 'components/form/FormContent'; import { TextOptionsInputGroup } from 'components/form/TextOptionsInputGroup'; import { FormButtonContainer } from 'components/form/FormButtonContainer'; import { selectCollectionType } from 'selectors/frontsSelectors'; +import { Criteria } from 'types/Grid'; const supportPortraitCropsSwitch = pageConfig?.userData?.featureSwitches.find( (feature) => feature.key === 'support-portrait-crops' @@ -75,7 +77,8 @@ const SUPPORT_PORTRAIT_CROPS = supportPortraitCropsSwitch ? supportPortraitCropsSwitch.enabled : false; -console.log({ SUPPORT_PORTRAIT_CROPS }); +//TO DO - get the right list of types, put it in the right place +const COLLECTIONS_USING_PORTRAIT_TRAILS = ['fixed/small/slow-IV']; interface ComponentProps extends ContainerProps { articleExists: boolean; @@ -697,7 +700,7 @@ class FormComponent extends React.Component { criteria={ isEditionsMode ? editionsCardImageCriteria - : defaultCardTrailImageCriteria + : this.determineCardCriteria() } frontId={frontId} defaultImageUrl={ @@ -946,6 +949,16 @@ class FormComponent extends React.Component { */ private getHeadlineLabel = () => this.props.snapType === 'html' ? 'Content' : 'Headline'; + + private determineCardCriteria = (): Criteria => { + const { collectionType } = this.props; + if (!SUPPORT_PORTRAIT_CROPS || !collectionType) { + return defaultCardTrailImageCriteria; + } + return COLLECTIONS_USING_PORTRAIT_TRAILS.includes(collectionType) + ? portraitCardImageCriteria + : landScapeCardImageCriteria; + }; } const CardForm = reduxForm({ From ed2ef42df2d0e625def34a3e5c46909223f1c55e Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Fri, 7 Jun 2024 08:44:29 +0100 Subject: [PATCH 12/35] dont change the FeatureSwitch class - just hide the new one client-side --- app/model/FeatureSwitches.scala | 18 ++++++------------ .../src/components/Features/FeaturesForm.tsx | 4 +++- fronts-client/src/types/Features.ts | 1 - 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/app/model/FeatureSwitches.scala b/app/model/FeatureSwitches.scala index ad059d081b9..341fd879e67 100644 --- a/app/model/FeatureSwitches.scala +++ b/app/model/FeatureSwitches.scala @@ -9,43 +9,37 @@ object FeatureSwitch { case class FeatureSwitch( key: String, title: String, - enabled: Boolean, - hideOnProd: Boolean + enabled: Boolean ) object ObscureFeed extends FeatureSwitch( key = "obscure-feed", title = "Obscure the feed -- it's distracting for developers!", - enabled = false, - hideOnProd = false + enabled = false ) object PageViewDataVisualisation extends FeatureSwitch( key = "page-view-data-visualisation", title = "Show page view data visualisation (aka spark lines)", - enabled = true, - hideOnProd = false + enabled = true ) object ShowFirefoxPrompt extends FeatureSwitch( key = "show-firefox-prompt", title = "Show the prompt to use Firefox if applicable", - enabled = false, - hideOnProd = false + enabled = false ) object TenImageSlideshows extends FeatureSwitch( key = "ten-image-slideshows", title = "Allow slideshows to contain 10 images rather than 5", - enabled = false, - hideOnProd = false + enabled = false ) object UsePortraitCropsForSomeCollectionTypes extends FeatureSwitch( key = "support-portrait-crops", title = "Use portrait crops for the experimental big card containers", - enabled = false, - hideOnProd = true + enabled = false ) object FeatureSwitches { diff --git a/fronts-client/src/components/Features/FeaturesForm.tsx b/fronts-client/src/components/Features/FeaturesForm.tsx index 144c2dabf81..671609d4989 100644 --- a/fronts-client/src/components/Features/FeaturesForm.tsx +++ b/fronts-client/src/components/Features/FeaturesForm.tsx @@ -15,8 +15,10 @@ interface Props { } const STAGE = pageConfig.env; +const SWITCHES_TO_HIDE_ON_PROD = ['support-portrait-crops']; + const filterSwitchesByStage = (featureSwitch: FeatureSwitch): boolean => - STAGE === 'code' || !featureSwitch.hideOnProd; + STAGE === 'code' || !SWITCHES_TO_HIDE_ON_PROD.includes(featureSwitch.key); class FeaturesForm extends React.Component { public render() { diff --git a/fronts-client/src/types/Features.ts b/fronts-client/src/types/Features.ts index c342935ed60..a47fe7bb0ee 100644 --- a/fronts-client/src/types/Features.ts +++ b/fronts-client/src/types/Features.ts @@ -2,5 +2,4 @@ export interface FeatureSwitch { key: string; title: string; enabled: boolean; - hideOnProd: boolean; } From 0431d36fc04a5599d9ed19193741e0e20f49a179 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Fri, 7 Jun 2024 09:25:35 +0100 Subject: [PATCH 13/35] provide criteria to slideshow --- fronts-client/src/components/form/ArticleMetaForm.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fronts-client/src/components/form/ArticleMetaForm.tsx b/fronts-client/src/components/form/ArticleMetaForm.tsx index ce257134e75..235d7fb3696 100644 --- a/fronts-client/src/components/form/ArticleMetaForm.tsx +++ b/fronts-client/src/components/form/ArticleMetaForm.tsx @@ -107,6 +107,7 @@ type RenderSlideshowProps = WrappedFieldArrayProps & { frontId: string; change: (field: string, value: any) => void; slideshowHasAtLeastTwoImages: boolean; + criteria: Criteria; }; const RowContainer = styled.div` @@ -243,6 +244,7 @@ const RenderSlideshow = ({ frontId, change, slideshowHasAtLeastTwoImages, + criteria, }: RenderSlideshowProps) => { const [slideshowIndex, setSlideshowIndex] = React.useState(0); @@ -301,8 +303,7 @@ const RenderSlideshow = ({ name={name} component={InputImage} small - // TO DO - will slideshows always be landscape? - criteria={landScapeCardImageCriteria} + criteria={criteria} frontId={frontId} isSelected={index === slideshowIndex} isInvalid={isInvalidCaptionLength(index)} @@ -810,6 +811,7 @@ class FormComponent extends React.Component { frontId={frontId} component={RenderSlideshow} change={change} + criteria={this.determineCardCriteria()} slideshowHasAtLeastTwoImages={slideshowHasAtLeastTwoImages} /> From e18ff3974b855ca792b308e07b1346f0c98218a3 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Fri, 7 Jun 2024 09:28:55 +0100 Subject: [PATCH 14/35] dont need state for imageDims - can get from props.input.value --- .../src/components/inputs/InputImage.tsx | 56 ++++++++----------- 1 file changed, 23 insertions(+), 33 deletions(-) diff --git a/fronts-client/src/components/inputs/InputImage.tsx b/fronts-client/src/components/inputs/InputImage.tsx index 9a19f49438d..52fc7a7f7f3 100644 --- a/fronts-client/src/components/inputs/InputImage.tsx +++ b/fronts-client/src/components/inputs/InputImage.tsx @@ -251,10 +251,6 @@ interface ComponentState { isDragging: boolean; modalOpen: boolean; imageSrc: string; - imageDims?: { - width: number; - height: number; - }; confirmDelete: boolean; cancelDeleteTimeout: undefined | (() => void); } @@ -274,21 +270,14 @@ class InputImage extends React.Component { ? (value as Record) : undefined; - const { src, height, width } = valueRecord ?? {}; + const { src } = valueRecord ?? {}; const imageSrc = typeof src === 'string' ? src : ''; - const imageDims = - typeof height === 'number' && typeof width === 'number' - ? { - height, - width, - } - : undefined; + this.state = { isDragging: false, modalOpen: false, imageSrc, - imageDims, confirmDelete: false, cancelDeleteTimeout: undefined, } as ComponentState; @@ -308,7 +297,8 @@ class InputImage extends React.Component { isSelected, isInvalid, } = this.props; - const { imageDims } = this.state; + + const imageDims = this.getCurrentImageDimensions(); if (!gridUrl) { return ( @@ -500,27 +490,16 @@ class InputImage extends React.Component { this.props.frontId, this.props.criteria ) - .then((mediaItem) => { - this.setState({ - imageDims: { - height: mediaItem.height, - width: mediaItem.width, - }, - }); - this.props.input.onChange(mediaItem); - }) + .then(this.props.input.onChange) .catch((err) => { alert(err); // tslint:disable-next-line no-console console.log('@todo:handle error', err); }); - this.setState({ imageSrc: '', imageDims: undefined }); + this.setState({ imageSrc: '' }); }; private clearField = () => { - this.setState({ - imageDims: undefined, - }); return this.props.input.onChange(null); }; @@ -559,12 +538,6 @@ class InputImage extends React.Component { ) .then((mediaItem) => { events.imageAdded(this.props.frontId, 'click to modal'); - this.setState({ - imageDims: { - width: mediaItem.width, - height: mediaItem.height, - }, - }); this.props.input.onChange(mediaItem); }) .catch((err) => { @@ -601,6 +574,23 @@ class InputImage extends React.Component { ? `${gridUrl}?cropType=portrait` : `${gridUrl}?cropType=landscape`; }; + + private getCurrentImageDimensions = () => { + const { value } = this.props.input; + const valueRecord = + value && typeof value === 'object' + ? (value as Record) + : undefined; + + const { height, width } = valueRecord ?? {}; + + return typeof height === 'number' && typeof width === 'number' + ? { + height, + width, + } + : undefined; + }; } const mapStateToProps = (state: State) => { From 9408df2b182122058e6bcba2e909c799147c78d0 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Fri, 7 Jun 2024 09:50:04 +0100 Subject: [PATCH 15/35] remove collection type from render --- fronts-client/src/components/inputs/InputImage.tsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/fronts-client/src/components/inputs/InputImage.tsx b/fronts-client/src/components/inputs/InputImage.tsx index 7368374e5a9..af6e92b6e58 100644 --- a/fronts-client/src/components/inputs/InputImage.tsx +++ b/fronts-client/src/components/inputs/InputImage.tsx @@ -274,7 +274,6 @@ class InputImage extends React.Component { const { src } = valueRecord ?? {}; const imageSrc = typeof src === 'string' ? src : ''; - this.state = { isDragging: false, modalOpen: false, @@ -297,7 +296,6 @@ class InputImage extends React.Component { disabled, isSelected, isInvalid, - collectionType, } = this.props; const imageDims = this.getCurrentImageDimensions(); @@ -332,7 +330,6 @@ class InputImage extends React.Component { isInvalid={isInvalid} confirmDelete={this.state.confirmDelete} > -

collectionType: {collectionType}

Date: Fri, 7 Jun 2024 10:50:22 +0100 Subject: [PATCH 16/35] use horrendous css funcions to style the image containers for small portraits in slideshows --- fronts-client/src/util/validateImageSrc.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fronts-client/src/util/validateImageSrc.ts b/fronts-client/src/util/validateImageSrc.ts index 0a42b72f0ff..ff5aed8cf75 100644 --- a/fronts-client/src/util/validateImageSrc.ts +++ b/fronts-client/src/util/validateImageSrc.ts @@ -186,6 +186,9 @@ function stripImplementationDetails( ) ) .then((gridImageJson: string) => + // TO DO - filtering here means getSuitableImageDetails only get crops that match + // criteria, so the error message ("The image does not have a valid crop on the Grid") + // is misleading - can be that it has valid crops but not matching criteria. filterGridCrops(gridImageJson, maybeFromGrid, criteria) ) .then((crops: Crop[]) => From 6e3ee6bcd2f1959bcd8db479247e4e8b6afa8172 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Fri, 7 Jun 2024 11:46:21 +0100 Subject: [PATCH 17/35] wrangle the css for ImageContainer --- .../src/components/inputs/InputImage.tsx | 64 +++++++++++++++---- 1 file changed, 50 insertions(+), 14 deletions(-) diff --git a/fronts-client/src/components/inputs/InputImage.tsx b/fronts-client/src/components/inputs/InputImage.tsx index af6e92b6e58..48d52927c78 100644 --- a/fronts-client/src/components/inputs/InputImage.tsx +++ b/fronts-client/src/components/inputs/InputImage.tsx @@ -36,6 +36,55 @@ import { selectEditMode } from '../../selectors/pathSelectors'; import CircularIconContainer from '../icons/CircularIconContainer'; import { error } from '../../styleConstants'; +const PORTRAIT_RATIO = 5 / 4; +const NORMAL_PORTRAIT_WIDTH = 160; +const SMALL_PORTRAIT_WIDTH = 60; +const TEXTINPUT_HEIGHT = 30; + +const smallPortaitStyle = ` + width: ${SMALL_PORTRAIT_WIDTH}px; + height: ${Math.floor(SMALL_PORTRAIT_WIDTH * PORTRAIT_RATIO)}px; + padding: 40% 0; + min-width: 50px; + margin: 0 auto; +`; + +const normalPortraitStyle = ` + width: ${NORMAL_PORTRAIT_WIDTH}px; + height: ${Math.floor( + NORMAL_PORTRAIT_WIDTH * PORTRAIT_RATIO + TEXTINPUT_HEIGHT + )}px; + `; + +const smallLandscapeStyle = ` + width: 100%; + maxWidth: 180px; + height: 0; + padding: 40%; + minWidth: 50px; +`; + +const normalLandscapeStyle = ` + width: 100%; + maxWidth: 180px; + height: 115px; +`; + +const getVariableImageContainerStyle = ({ + portrait = false, + small = false, +}: { + small?: boolean; + portrait?: boolean; +}) => + portrait + ? small + ? smallPortaitStyle + : normalPortraitStyle + : small + ? smallLandscapeStyle + : normalLandscapeStyle; + // assuming any portrait image (ie height>width) // is in the 4:5 ratio for purposes of styling // the image container @@ -46,21 +95,8 @@ const ImageContainer = styled.div<{ display: flex; flex-direction: column; position: relative; - width: 100%; - max-width: ${(props) => !props.small && '180px'}; - ${({ small }) => - small && - `min-width: 50px; - padding: 40%;`} - height: ${(props) => (props.small ? '0' : '115px')}; transition: background-color 0.15s; - - ${({ portrait, small }) => - portrait && - ` - width: ${small ? 50 : 200}px; - height: ${small ? 62 : 250}px; - `} + ${getVariableImageContainerStyle} `; const AddImageButton = styled(ButtonDefault)<{ small?: boolean }>` From 4299278f11a8fe164456fccbfa5370605f21bfc6 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Fri, 7 Jun 2024 11:51:50 +0100 Subject: [PATCH 18/35] move the image container to own file --- .../image/ImageInputImageContainer.tsx | 64 +++++++++++++++++++ .../src/components/inputs/InputImage.tsx | 64 +------------------ 2 files changed, 65 insertions(+), 63 deletions(-) create mode 100644 fronts-client/src/components/image/ImageInputImageContainer.tsx diff --git a/fronts-client/src/components/image/ImageInputImageContainer.tsx b/fronts-client/src/components/image/ImageInputImageContainer.tsx new file mode 100644 index 00000000000..f5cbea90713 --- /dev/null +++ b/fronts-client/src/components/image/ImageInputImageContainer.tsx @@ -0,0 +1,64 @@ +import { styled } from 'constants/theme'; + +const PORTRAIT_RATIO = 5 / 4; +const NORMAL_PORTRAIT_WIDTH = 160; +const SMALL_PORTRAIT_WIDTH = 60; +const TEXTINPUT_HEIGHT = 30; + +const smallPortaitStyle = ` + width: ${SMALL_PORTRAIT_WIDTH}px; + height: ${Math.floor(SMALL_PORTRAIT_WIDTH * PORTRAIT_RATIO)}px; + padding: 40% 0; + min-width: 50px; + margin: 0 auto; +`; + +const normalPortraitStyle = ` + width: ${NORMAL_PORTRAIT_WIDTH}px; + height: ${Math.floor( + NORMAL_PORTRAIT_WIDTH * PORTRAIT_RATIO + TEXTINPUT_HEIGHT + )}px; + `; + +const smallLandscapeStyle = ` + width: 100%; + maxWidth: 180px; + height: 0; + padding: 40%; + minWidth: 50px; +`; + +const normalLandscapeStyle = ` + width: 100%; + maxWidth: 180px; + height: 115px; +`; + +const getVariableImageContainerStyle = ({ + portrait = false, + small = false, +}: { + small?: boolean; + portrait?: boolean; +}) => + portrait + ? small + ? smallPortaitStyle + : normalPortraitStyle + : small + ? smallLandscapeStyle + : normalLandscapeStyle; + +// assuming any portrait image (ie height>width) +// is in the 4:5 ratio for purposes of styling +// the image container +export const ImageInputImageContainer = styled.div<{ + small?: boolean; + portrait?: boolean; +}>` + display: flex; + flex-direction: column; + position: relative; + transition: background-color 0.15s; + ${getVariableImageContainerStyle} +`; diff --git a/fronts-client/src/components/inputs/InputImage.tsx b/fronts-client/src/components/inputs/InputImage.tsx index 48d52927c78..dbe8ae1cfa5 100644 --- a/fronts-client/src/components/inputs/InputImage.tsx +++ b/fronts-client/src/components/inputs/InputImage.tsx @@ -31,74 +31,12 @@ import { portraitCardImageCriteria, } from 'constants/image'; import ImageDragIntentIndicator from 'components/image/ImageDragIntentIndicator'; +import { ImageInputImageContainer as ImageContainer } from 'components/image/ImageInputImageContainer'; import { EditMode } from 'types/EditMode'; import { selectEditMode } from '../../selectors/pathSelectors'; import CircularIconContainer from '../icons/CircularIconContainer'; import { error } from '../../styleConstants'; -const PORTRAIT_RATIO = 5 / 4; -const NORMAL_PORTRAIT_WIDTH = 160; -const SMALL_PORTRAIT_WIDTH = 60; -const TEXTINPUT_HEIGHT = 30; - -const smallPortaitStyle = ` - width: ${SMALL_PORTRAIT_WIDTH}px; - height: ${Math.floor(SMALL_PORTRAIT_WIDTH * PORTRAIT_RATIO)}px; - padding: 40% 0; - min-width: 50px; - margin: 0 auto; -`; - -const normalPortraitStyle = ` - width: ${NORMAL_PORTRAIT_WIDTH}px; - height: ${Math.floor( - NORMAL_PORTRAIT_WIDTH * PORTRAIT_RATIO + TEXTINPUT_HEIGHT - )}px; - `; - -const smallLandscapeStyle = ` - width: 100%; - maxWidth: 180px; - height: 0; - padding: 40%; - minWidth: 50px; -`; - -const normalLandscapeStyle = ` - width: 100%; - maxWidth: 180px; - height: 115px; -`; - -const getVariableImageContainerStyle = ({ - portrait = false, - small = false, -}: { - small?: boolean; - portrait?: boolean; -}) => - portrait - ? small - ? smallPortaitStyle - : normalPortraitStyle - : small - ? smallLandscapeStyle - : normalLandscapeStyle; - -// assuming any portrait image (ie height>width) -// is in the 4:5 ratio for purposes of styling -// the image container -const ImageContainer = styled.div<{ - small?: boolean; - portrait?: boolean; -}>` - display: flex; - flex-direction: column; - position: relative; - transition: background-color 0.15s; - ${getVariableImageContainerStyle} -`; - const AddImageButton = styled(ButtonDefault)<{ small?: boolean }>` background-color: ${({ small }) => small ? theme.colors.greyLight : `#5e5e5e50`}; From e3e7df4cea431b650e5ba01b432f42a361a00553 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Fri, 7 Jun 2024 12:38:09 +0100 Subject: [PATCH 19/35] validate image error distinguishes between 'no crops' and 'no matching crops' --- fronts-client/src/util/validateImageSrc.ts | 39 ++++++++++++++++------ 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/fronts-client/src/util/validateImageSrc.ts b/fronts-client/src/util/validateImageSrc.ts index ff5aed8cf75..5411ae70abe 100644 --- a/fronts-client/src/util/validateImageSrc.ts +++ b/fronts-client/src/util/validateImageSrc.ts @@ -67,12 +67,14 @@ function getSuitableImageDetails( id: string, desired: Criteria ): Promise { + const { maxWidth, minWidth, widthAspectRatio, heightAspectRatio } = desired; if (crops.length === 0) { return Promise.reject( - new Error('The image does not have a valid crop on the Grid') + new Error( + `The image does not have a valid ${widthAspectRatio}:${heightAspectRatio} crop on the Grid` + ) ); } - const { maxWidth, minWidth } = desired; const assets = sortBy( [crops[0].master] .concat(crops[0].assets) @@ -185,14 +187,31 @@ function stripImplementationDetails( new Error(`There was a problem contacting The Grid - ${e.message}`) ) ) - .then((gridImageJson: string) => - // TO DO - filtering here means getSuitableImageDetails only get crops that match - // criteria, so the error message ("The image does not have a valid crop on the Grid") - // is misleading - can be that it has valid crops but not matching criteria. - filterGridCrops(gridImageJson, maybeFromGrid, criteria) - ) - .then((crops: Crop[]) => - getSuitableImageDetails(crops, maybeFromGrid.id, criteria || {}) + .then((gridImageJson: string) => ({ + crops: filterGridCrops(gridImageJson, maybeFromGrid, criteria), + areNoCropsOfAnySize: + filterGridCrops(gridImageJson, maybeFromGrid).length === 0, + })) + .then( + ({ + crops, + areNoCropsOfAnySize, + }: { + crops: Crop[]; + areNoCropsOfAnySize: boolean; + }) => { + if (crops.length === 0 && areNoCropsOfAnySize) { + return Promise.reject( + new Error('The image does not have any crops on the Grid') + ); + } + + return getSuitableImageDetails( + crops, + maybeFromGrid.id, + criteria || {} + ); + } ) .then((asset: ImageDescription) => resolve({ From 558c4abb61c0342c0a31199f6729ee3d6b939221 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Mon, 10 Jun 2024 09:03:32 +0100 Subject: [PATCH 20/35] dont use the default criteria --- fronts-client/src/components/form/ArticleMetaForm.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fronts-client/src/components/form/ArticleMetaForm.tsx b/fronts-client/src/components/form/ArticleMetaForm.tsx index 235d7fb3696..ca68bc55495 100644 --- a/fronts-client/src/components/form/ArticleMetaForm.tsx +++ b/fronts-client/src/components/form/ArticleMetaForm.tsx @@ -47,7 +47,6 @@ import { editionsCardImageCriteria, editionsMobileCardImageCriteria, editionsTabletCardImageCriteria, - defaultCardTrailImageCriteria, portraitCardImageCriteria, } from 'constants/image'; import { selectors as collectionSelectors } from 'bundles/collectionsBundle'; @@ -955,7 +954,7 @@ class FormComponent extends React.Component { private determineCardCriteria = (): Criteria => { const { collectionType } = this.props; if (!SUPPORT_PORTRAIT_CROPS || !collectionType) { - return defaultCardTrailImageCriteria; + return landScapeCardImageCriteria; } return COLLECTIONS_USING_PORTRAIT_TRAILS.includes(collectionType) ? portraitCardImageCriteria From 9807964e70b7a6819ef28ffb342d1a438b8bf9df Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Mon, 10 Jun 2024 09:56:43 +0100 Subject: [PATCH 21/35] move constants to constants/image --- .../src/components/form/ArticleMetaForm.tsx | 12 ++---------- fronts-client/src/constants/image.ts | 10 ++++++++++ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/fronts-client/src/components/form/ArticleMetaForm.tsx b/fronts-client/src/components/form/ArticleMetaForm.tsx index ca68bc55495..941bb19a8a2 100644 --- a/fronts-client/src/components/form/ArticleMetaForm.tsx +++ b/fronts-client/src/components/form/ArticleMetaForm.tsx @@ -48,6 +48,8 @@ import { editionsMobileCardImageCriteria, editionsTabletCardImageCriteria, portraitCardImageCriteria, + COLLECTIONS_USING_PORTRAIT_TRAILS, + SUPPORT_PORTRAIT_CROPS, } from 'constants/image'; import { selectors as collectionSelectors } from 'bundles/collectionsBundle'; import { getContributorImage } from 'util/CAPIUtils'; @@ -68,16 +70,6 @@ import { FormButtonContainer } from 'components/form/FormButtonContainer'; import { selectCollectionType } from 'selectors/frontsSelectors'; import { Criteria } from 'types/Grid'; -const supportPortraitCropsSwitch = pageConfig?.userData?.featureSwitches.find( - (feature) => feature.key === 'support-portrait-crops' -); - -const SUPPORT_PORTRAIT_CROPS = supportPortraitCropsSwitch - ? supportPortraitCropsSwitch.enabled - : false; - -//TO DO - get the right list of types, put it in the right place -const COLLECTIONS_USING_PORTRAIT_TRAILS = ['fixed/small/slow-IV']; interface ComponentProps extends ContainerProps { articleExists: boolean; diff --git a/fronts-client/src/constants/image.ts b/fronts-client/src/constants/image.ts index e3dfb2d7b46..b9fe5d1869f 100644 --- a/fronts-client/src/constants/image.ts +++ b/fronts-client/src/constants/image.ts @@ -1,3 +1,10 @@ +import pageConfig from 'util/extractConfigFromPage'; + +export const SUPPORT_PORTRAIT_CROPS = + pageConfig?.userData?.featureSwitches.find( + (feature) => feature.key === 'support-portrait-crops' + )?.enabled || false; + export const landScapeCardImageCriteria = { minWidth: 400, widthAspectRatio: 5, @@ -10,6 +17,9 @@ export const portraitCardImageCriteria = { heightAspectRatio: 5, }; +//TO DO - get the right list of types +export const COLLECTIONS_USING_PORTRAIT_TRAILS = ['fixed/small/slow-IV']; + export const defaultCardTrailImageCriteria = landScapeCardImageCriteria; export const editionsCardImageCriteria = { From 07a6d271419edb333e55fa8c56648fd479870a4c Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Mon, 10 Jun 2024 10:33:25 +0100 Subject: [PATCH 22/35] Card components check the collection type to set criteria for image drops --- fronts-client/src/components/card/Card.tsx | 30 ++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/fronts-client/src/components/card/Card.tsx b/fronts-client/src/components/card/Card.tsx index 6cab6b3db14..af3d4b44f7f 100644 --- a/fronts-client/src/components/card/Card.tsx +++ b/fronts-client/src/components/card/Card.tsx @@ -23,6 +23,10 @@ import { import { editionsCardImageCriteria, DRAG_DATA_CARD_IMAGE_OVERRIDE, + SUPPORT_PORTRAIT_CROPS, + COLLECTIONS_USING_PORTRAIT_TRAILS, + landScapeCardImageCriteria, + portraitCardImageCriteria, defaultCardTrailImageCriteria, } from 'constants/image'; import Sublinks from '../FrontsEdit/CollectionComponents/Sublinks'; @@ -46,6 +50,8 @@ import { CardTypes, CardTypesMap } from 'constants/cardTypes'; import { RecipeCard } from 'components/card/recipe/RecipeCard'; import { ChefCard } from 'components/card/chef/ChefCard'; import { ChefMetaForm } from '../form/ChefMetaForm'; +import { selectCollectionType } from 'selectors/frontsSelectors'; +import { Criteria } from 'types/Grid'; export const createCardId = (id: string) => `collection-item-${id}`; @@ -104,6 +110,7 @@ type CardContainerProps = ContainerProps & { editMode: EditMode; isLive?: boolean; pillarId?: string; + collectionType?: string; }; class Card extends React.Component { @@ -346,7 +353,7 @@ class Card extends React.Component { const isEditionsMode = this.props.editMode === 'editions'; const imageCriteria = isEditionsMode ? editionsCardImageCriteria - : defaultCardTrailImageCriteria; + : this.determineCardCriteria(); // Our drag contains Grid data validateImageEvent(e, this.props.frontId, imageCriteria) @@ -355,11 +362,29 @@ class Card extends React.Component { ) .catch(alert); }; + + private determineCardCriteria = (): Criteria => { + const { collectionType, parentId } = this.props; + + // TO DO - how to handle image drags to a clipboard card? + // ask the user what type they want with a modal? + if (parentId === 'clipboard') { + return defaultCardTrailImageCriteria; + } + + if (!SUPPORT_PORTRAIT_CROPS || !collectionType) { + return landScapeCardImageCriteria; + } + + return COLLECTIONS_USING_PORTRAIT_TRAILS.includes(collectionType) + ? portraitCardImageCriteria + : landScapeCardImageCriteria; + }; } const createMapStateToProps = () => { const selectType = createSelectCardType(); - return (state: State, { uuid, frontId }: ContainerProps) => { + return (state: State, { uuid, frontId, collectionId }: ContainerProps) => { const maybeExternalArticle = selectExternalArticleFromCard(state, uuid); return { type: selectType(state, uuid), @@ -368,6 +393,7 @@ const createMapStateToProps = () => { pillarId: maybeExternalArticle && maybeExternalArticle.pillarId, numSupportingArticles: selectSupportingArticleCount(state, uuid), editMode: selectEditMode(state), + collectionType: collectionId && selectCollectionType(state, collectionId), }; }; }; From 3eb882c2ceaf31d685d95ea195565aea2583cc94 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Mon, 10 Jun 2024 11:21:50 +0100 Subject: [PATCH 23/35] add TO DO about dragging images from card to card --- fronts-client/src/components/card/Card.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fronts-client/src/components/card/Card.tsx b/fronts-client/src/components/card/Card.tsx index af3d4b44f7f..6fe3c7a6d8b 100644 --- a/fronts-client/src/components/card/Card.tsx +++ b/fronts-client/src/components/card/Card.tsx @@ -346,6 +346,10 @@ class Card extends React.Component { // Our drag is a copy event, from another Card const cardUuid = e.dataTransfer.getData(DRAG_DATA_CARD_IMAGE_OVERRIDE); if (cardUuid) { + // TO DO - before copying from another card, check its image + // matches this card's collection's criteria. + // ideally - if they don't match, check grid for a matching + // crop on the image and use that if present! this.props.copyCardImageMeta(cardUuid, this.props.uuid); return; } From 0f998a67d958605bf353c659c6b5fa31a671edb1 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Mon, 10 Jun 2024 12:12:12 +0100 Subject: [PATCH 24/35] cards check replacement image dimensions and style thumbnails to display portraits right --- .../components/card/article/ArticleBody.tsx | 20 +++++++++++++++++++ .../src/components/image/Thumbnail.tsx | 10 ++++++++++ 2 files changed, 30 insertions(+) diff --git a/fronts-client/src/components/card/article/ArticleBody.tsx b/fronts-client/src/components/card/article/ArticleBody.tsx index 666b4191c85..13df17d32a8 100644 --- a/fronts-client/src/components/card/article/ArticleBody.tsx +++ b/fronts-client/src/components/card/article/ArticleBody.tsx @@ -86,6 +86,15 @@ const ClipboardFirstPublished = styled.div` right: 0; `; +const getThumbnailDims = (imageSrcWidth?: string, imageSrcHeight?: string) => { + if (!imageSrcHeight || !imageSrcWidth) { + return undefined; + } + const height = Number(imageSrcHeight); + const width = Number(imageSrcWidth); + return isNaN(height) || isNaN(width) ? undefined : { width, height }; +}; + interface ArticleBodyProps { newspaperPageNumber?: number; @@ -131,6 +140,8 @@ interface ArticleBodyProps { canShowPageViewData: boolean; frontId: string; collectionId?: string; + imageSrcWidth?: string; + imageSrcHeight?: string; } const articleBodyDefault = React.memo( @@ -176,11 +187,19 @@ const articleBodyDefault = React.memo( collectionId, newspaperPageNumber, promotionMetric, + imageSrcWidth, + imageSrcHeight, }: ArticleBodyProps) => { const displayByline = size === 'default' && showByline && byline; const now = Date.now(); const paths = urlPath ? getPaths(urlPath) : undefined; + const thumbnailDims = getThumbnailDims(imageSrcWidth, imageSrcHeight); + const thumbnailIsPortrait = + !!imageReplace && + thumbnailDims && + thumbnailDims.height > thumbnailDims.width; + return ( <> {showMeta && ( @@ -294,6 +313,7 @@ const articleBodyDefault = React.memo( imageHide={imageHide} url={thumbnail} isDraggingImageOver={isDraggingImageOver} + isPortrait={thumbnailIsPortrait} > {cutoutThumbnail ? ( diff --git a/fronts-client/src/components/image/Thumbnail.tsx b/fronts-client/src/components/image/Thumbnail.tsx index e52604c8264..404a7ad495d 100644 --- a/fronts-client/src/components/image/Thumbnail.tsx +++ b/fronts-client/src/components/image/Thumbnail.tsx @@ -9,6 +9,7 @@ const ThumbnailSmall = styled(ThumbnailBase)<{ url?: string | void; isDraggingImageOver?: boolean; imageHide?: boolean; + isPortrait?: boolean; }>` position: relative; width: ${theme.thumbnailImage.width}; @@ -19,6 +20,15 @@ const ThumbnailSmall = styled(ThumbnailBase)<{ font-weight: bold; opacity: ${({ imageHide }) => (imageHide && imageHide ? '0.5' : '1')}; background-image: ${({ url }) => `url('${url}')`}; + + ${({ isPortrait }) => + isPortrait && + ` + background-size: contain; + background-repeat: no-repeat; + background-position-x: center; + `} + ${({ isDraggingImageOver }) => isDraggingImageOver && `background: ${theme.base.colors.dropZoneActiveStory}; From 57440c966f784f3373e77a884573ca94636a1b58 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Mon, 10 Jun 2024 14:39:43 +0100 Subject: [PATCH 25/35] validate dimensions of incoming dragged images from other cards --- fronts-client/src/components/card/Card.tsx | 68 +++++++++++++++---- .../components/card/article/ArticleBody.tsx | 15 ++-- fronts-client/src/constants/image.ts | 2 +- fronts-client/src/util/validateImageSrc.ts | 49 +++++++++++++ 4 files changed, 111 insertions(+), 23 deletions(-) diff --git a/fronts-client/src/components/card/Card.tsx b/fronts-client/src/components/card/Card.tsx index 6fe3c7a6d8b..067567df887 100644 --- a/fronts-client/src/components/card/Card.tsx +++ b/fronts-client/src/components/card/Card.tsx @@ -5,6 +5,7 @@ import Article from 'components/card/article/ArticleCard'; import type { State } from 'types/State'; import { createSelectCardType } from 'selectors/cardSelectors'; import { + selectCard, selectExternalArticleFromCard, selectSupportingArticleCount, } from 'selectors/shared'; @@ -17,6 +18,8 @@ import { } from 'actions/Cards'; import { dragEventHasImageData, + getMaybeDimensionsFromWidthAndHeight, + validateDimensions, validateImageEvent, ValidationResponse, } from 'util/validateImageSrc'; @@ -52,6 +55,7 @@ import { ChefCard } from 'components/card/chef/ChefCard'; import { ChefMetaForm } from '../form/ChefMetaForm'; import { selectCollectionType } from 'selectors/frontsSelectors'; import { Criteria } from 'types/Grid'; +import { Card as CardType } from 'types/Collection'; export const createCardId = (id: string) => `collection-item-${id}`; @@ -111,6 +115,7 @@ type CardContainerProps = ContainerProps & { isLive?: boolean; pillarId?: string; collectionType?: string; + selectOtherCard: { (uuid: string): CardType }; }; class Card extends React.Component { @@ -343,22 +348,35 @@ class Card extends React.Component { e.preventDefault(); e.persist(); + const isEditionsMode = this.props.editMode === 'editions'; + const imageCriteria = isEditionsMode + ? editionsCardImageCriteria + : this.determineCardCriteria(); + // Our drag is a copy event, from another Card const cardUuid = e.dataTransfer.getData(DRAG_DATA_CARD_IMAGE_OVERRIDE); if (cardUuid) { - // TO DO - before copying from another card, check its image - // matches this card's collection's criteria. - // ideally - if they don't match, check grid for a matching - // crop on the image and use that if present! + if (!isEditionsMode) { + // check dragged image matches this card's collection's criteria. + const validationForDraggedImage = this.checkDraggedImage( + cardUuid, + imageCriteria + ); + if (!validationForDraggedImage.matchesCriteria) { + // @todo - if they don't match, check grid for a matching + // crop on the image and use that if present? + // @todo handle error + alert( + `Cannot copy that image to this card: ${validationForDraggedImage.reason}` + ); + return; + } + } + this.props.copyCardImageMeta(cardUuid, this.props.uuid); return; } - const isEditionsMode = this.props.editMode === 'editions'; - const imageCriteria = isEditionsMode - ? editionsCardImageCriteria - : this.determineCardCriteria(); - // Our drag contains Grid data validateImageEvent(e, this.props.frontId, imageCriteria) .then((imageData) => @@ -369,9 +387,12 @@ class Card extends React.Component { private determineCardCriteria = (): Criteria => { const { collectionType, parentId } = this.props; - - // TO DO - how to handle image drags to a clipboard card? - // ask the user what type they want with a modal? + // @todo - how best to handle image drags to a clipboard card? + // Ask the user what type they want with a modal? + // ideally, should resolve the conflict when the clipboard item + // is dragged to a collection, not force the user to decide in advance. + // For using the default (landscape) + // since that is what all live containers use. if (parentId === 'clipboard') { return defaultCardTrailImageCriteria; } @@ -384,6 +405,27 @@ class Card extends React.Component { ? portraitCardImageCriteria : landScapeCardImageCriteria; }; + + private checkDraggedImage = ( + cardUuid: string, + imageCriteria: Criteria + ): ReturnType => { + // check dragged image matches this card's collection's criteria. + const cardImageWasDraggedFrom = this.props.selectOtherCard(cardUuid); + + const draggedImageDims = getMaybeDimensionsFromWidthAndHeight( + cardImageWasDraggedFrom?.meta?.imageSrcWidth, + cardImageWasDraggedFrom?.meta?.imageSrcHeight + ); + + if (!draggedImageDims) { + return { + matchesCriteria: false, + reason: 'no replacement image found', + }; + } + return validateDimensions(draggedImageDims, imageCriteria); + }; } const createMapStateToProps = () => { @@ -398,6 +440,8 @@ const createMapStateToProps = () => { numSupportingArticles: selectSupportingArticleCount(state, uuid), editMode: selectEditMode(state), collectionType: collectionId && selectCollectionType(state, collectionId), + + selectOtherCard: (uuid: string) => selectCard(state, uuid), }; }; }; diff --git a/fronts-client/src/components/card/article/ArticleBody.tsx b/fronts-client/src/components/card/article/ArticleBody.tsx index 13df17d32a8..41135de3b77 100644 --- a/fronts-client/src/components/card/article/ArticleBody.tsx +++ b/fronts-client/src/components/card/article/ArticleBody.tsx @@ -34,6 +34,7 @@ import EditModeVisibility from 'components/util/EditModeVisibility'; import PageViewDataWrapper from '../../PageViewDataWrapper'; import ImageAndGraphWrapper from 'components/image/ImageAndGraphWrapper'; import { getPaths } from 'util/paths'; +import { getMaybeDimensionsFromWidthAndHeight } from 'util/validateImageSrc'; const ThumbnailPlaceholder = styled(BasePlaceholder)` flex-shrink: 0; @@ -86,15 +87,6 @@ const ClipboardFirstPublished = styled.div` right: 0; `; -const getThumbnailDims = (imageSrcWidth?: string, imageSrcHeight?: string) => { - if (!imageSrcHeight || !imageSrcWidth) { - return undefined; - } - const height = Number(imageSrcHeight); - const width = Number(imageSrcWidth); - return isNaN(height) || isNaN(width) ? undefined : { width, height }; -}; - interface ArticleBodyProps { newspaperPageNumber?: number; @@ -194,7 +186,10 @@ const articleBodyDefault = React.memo( const now = Date.now(); const paths = urlPath ? getPaths(urlPath) : undefined; - const thumbnailDims = getThumbnailDims(imageSrcWidth, imageSrcHeight); + const thumbnailDims = getMaybeDimensionsFromWidthAndHeight( + imageSrcWidth, + imageSrcHeight + ); const thumbnailIsPortrait = !!imageReplace && thumbnailDims && diff --git a/fronts-client/src/constants/image.ts b/fronts-client/src/constants/image.ts index b9fe5d1869f..5ff0a6e191a 100644 --- a/fronts-client/src/constants/image.ts +++ b/fronts-client/src/constants/image.ts @@ -17,7 +17,7 @@ export const portraitCardImageCriteria = { heightAspectRatio: 5, }; -//TO DO - get the right list of types +// @todo - add the right collection type when it exists export const COLLECTIONS_USING_PORTRAIT_TRAILS = ['fixed/small/slow-IV']; export const defaultCardTrailImageCriteria = landScapeCardImageCriteria; diff --git a/fronts-client/src/util/validateImageSrc.ts b/fronts-client/src/util/validateImageSrc.ts index 5411ae70abe..da88b526b79 100644 --- a/fronts-client/src/util/validateImageSrc.ts +++ b/fronts-client/src/util/validateImageSrc.ts @@ -321,6 +321,53 @@ function validateImageEvent( ); } +const getMaybeDimensionsFromWidthAndHeight = ( + imageSrcWidth?: string | number, + imageSrcHeight?: string | number +) => { + if (!imageSrcHeight || !imageSrcWidth) { + return undefined; + } + const height = Number(imageSrcHeight); + const width = Number(imageSrcWidth); + return isNaN(height) || isNaN(width) ? undefined : { width, height }; +}; + +function validateDimensions( + dimensions: { width: number; height: number }, + criteria: Criteria +): { matchesCriteria: true } | { matchesCriteria: false; reason: string } { + const { width, height } = dimensions; + const { maxWidth, minWidth, widthAspectRatio, heightAspectRatio } = criteria; + const criteriaRatio = + widthAspectRatio && heightAspectRatio + ? widthAspectRatio / heightAspectRatio + : NaN; + + const ratio = width / height; + + if (maxWidth && maxWidth < width) { + return { + matchesCriteria: false, + reason: `Images cannot be more than ${maxWidth} pixels wide`, + }; + } else if (minWidth && minWidth > width) { + return { + matchesCriteria: false, + reason: `Images cannot be less than ${minWidth} pixels wide`, + }; + } else if (criteriaRatio && Math.abs(criteriaRatio - ratio) > 0.01) { + return { + matchesCriteria: false, + reason: `Images must have a ${widthAspectRatio || ''}:${ + heightAspectRatio || '' + } aspect ratio`, + }; + } + + return { matchesCriteria: true }; +} + const imageDropTypes = [ ...gridDropTypes, DRAG_DATA_CARD_IMAGE_OVERRIDE, @@ -338,4 +385,6 @@ export { validateImageSrc, validateImageEvent, validateMediaItem, + validateDimensions, + getMaybeDimensionsFromWidthAndHeight, }; From 936e9c6e62ce8e6608606e150ea1ac8c12d8d6f6 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Mon, 10 Jun 2024 17:29:54 +0100 Subject: [PATCH 26/35] undo switch change --- app/model/FeatureSwitches.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/model/FeatureSwitches.scala b/app/model/FeatureSwitches.scala index 341fd879e67..bfdb2351412 100644 --- a/app/model/FeatureSwitches.scala +++ b/app/model/FeatureSwitches.scala @@ -27,7 +27,7 @@ object PageViewDataVisualisation extends FeatureSwitch( object ShowFirefoxPrompt extends FeatureSwitch( key = "show-firefox-prompt", title = "Show the prompt to use Firefox if applicable", - enabled = false + enabled = true ) object TenImageSlideshows extends FeatureSwitch( From c3b52abf6847d0e3461fd3468392ff9254961fc1 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Mon, 10 Jun 2024 17:30:21 +0100 Subject: [PATCH 27/35] more coherent comments --- .../src/components/Features/FeaturesForm.tsx | 5 +++++ fronts-client/src/components/card/Card.tsx | 16 ++++++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/fronts-client/src/components/Features/FeaturesForm.tsx b/fronts-client/src/components/Features/FeaturesForm.tsx index 671609d4989..b1bde6b71f7 100644 --- a/fronts-client/src/components/Features/FeaturesForm.tsx +++ b/fronts-client/src/components/Features/FeaturesForm.tsx @@ -15,6 +15,11 @@ interface Props { } const STAGE = pageConfig.env; + +// We don't yet have any collectionTypes that use portrait crops +// but even when we do, we might not want to show the option on PROD +// it might lead to some broken visuals if used before implemented +// on platforms. const SWITCHES_TO_HIDE_ON_PROD = ['support-portrait-crops']; const filterSwitchesByStage = (featureSwitch: FeatureSwitch): boolean => diff --git a/fronts-client/src/components/card/Card.tsx b/fronts-client/src/components/card/Card.tsx index 067567df887..23afd79fcff 100644 --- a/fronts-client/src/components/card/Card.tsx +++ b/fronts-client/src/components/card/Card.tsx @@ -364,7 +364,7 @@ class Card extends React.Component { ); if (!validationForDraggedImage.matchesCriteria) { // @todo - if they don't match, check grid for a matching - // crop on the image and use that if present? + // crop of the image and use that if present? // @todo handle error alert( `Cannot copy that image to this card: ${validationForDraggedImage.reason}` @@ -387,12 +387,13 @@ class Card extends React.Component { private determineCardCriteria = (): Criteria => { const { collectionType, parentId } = this.props; - // @todo - how best to handle image drags to a clipboard card? - // Ask the user what type they want with a modal? - // ideally, should resolve the conflict when the clipboard item - // is dragged to a collection, not force the user to decide in advance. - // For using the default (landscape) - // since that is what all live containers use. + // @todo - how best to handle crop drags to a clipboard card? + // Using the default (landscape) for now. + // But, if you set a replacement (lanscape) trail on a clipboard + // item, that item can't be dragged to a portrit collection. + // Ideally, handleImageDrop will check if the Image has a matching + // crop of the required criteria and use that instead of the crop + // being dragged (or the crop on the card being dragged) onto the card if (parentId === 'clipboard') { return defaultCardTrailImageCriteria; } @@ -440,7 +441,6 @@ const createMapStateToProps = () => { numSupportingArticles: selectSupportingArticleCount(state, uuid), editMode: selectEditMode(state), collectionType: collectionId && selectCollectionType(state, collectionId), - selectOtherCard: (uuid: string) => selectCard(state, uuid), }; }; From 1f9c9ac9b7e6f4881feda37a31e8970f1651a47d Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Tue, 11 Jun 2024 16:50:07 +0100 Subject: [PATCH 28/35] update tests to account for more specific error messaging --- .../src/util/__tests__/validateImageSrc.spec.ts | 10 +++++----- fronts-client/src/util/validateImageSrc.ts | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/fronts-client/src/util/__tests__/validateImageSrc.spec.ts b/fronts-client/src/util/__tests__/validateImageSrc.spec.ts index cf1b064c911..206f6a21ad5 100644 --- a/fronts-client/src/util/__tests__/validateImageSrc.spec.ts +++ b/fronts-client/src/util/__tests__/validateImageSrc.spec.ts @@ -213,7 +213,7 @@ describe('Validate images', () => { ).then( (err) => done.fail(err.toString()), (err) => { - expect(err.message).toMatch(/does not have a valid crop/i); + expect(err.message).toMatch(/does not have any valid crops/i); done(); } ); @@ -242,13 +242,13 @@ describe('Validate images', () => { { minWidth: 100, maxWidth: 1000, - widthAspectRatio: 5, + widthAspectRatio: 7, heightAspectRatio: 4, } ).then( (err) => done.fail(err.toString()), (err) => { - expect(err.message).toMatch(/does not have a valid crop/i); + expect(err.message).toMatch(/does not have a valid 7:4 crop/i); done(); } ); @@ -311,7 +311,7 @@ describe('Validate images', () => { ).then( (err) => done.fail(err.toString()), (err) => { - expect(err.message).toMatch(/does not have a valid crop/i); + expect(err.message).toMatch(/does not have any valid crops/i); done(); } ); @@ -348,7 +348,7 @@ describe('Validate images', () => { ).then( (err) => done.fail(err.toString()), (err) => { - expect(err.message).toMatch(/does not have a valid crop/i); + expect(err.message).toMatch(/does not have a valid 5:4 crop/i); done(); } ); diff --git a/fronts-client/src/util/validateImageSrc.ts b/fronts-client/src/util/validateImageSrc.ts index da88b526b79..818a1a91e95 100644 --- a/fronts-client/src/util/validateImageSrc.ts +++ b/fronts-client/src/util/validateImageSrc.ts @@ -202,7 +202,7 @@ function stripImplementationDetails( }) => { if (crops.length === 0 && areNoCropsOfAnySize) { return Promise.reject( - new Error('The image does not have any crops on the Grid') + new Error('The image does not have any valid crops on the Grid') ); } From 838694ced2a30eecea37fc7e1c4ae827b5bf7433 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Tue, 11 Jun 2024 17:49:33 +0100 Subject: [PATCH 29/35] message correctly when crops fail validation of a criteria without aspect ratio --- .../util/__tests__/validateImageSrc.spec.ts | 36 ++++++++++++++++++- fronts-client/src/util/validateImageSrc.ts | 5 ++- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/fronts-client/src/util/__tests__/validateImageSrc.spec.ts b/fronts-client/src/util/__tests__/validateImageSrc.spec.ts index 206f6a21ad5..faab4b6c01b 100644 --- a/fronts-client/src/util/__tests__/validateImageSrc.spec.ts +++ b/fronts-client/src/util/__tests__/validateImageSrc.spec.ts @@ -317,7 +317,7 @@ describe('Validate images', () => { ); }); - it("fails if crops don't respect criteria", (done) => { + it("fails if crops don't respect criteria with an aspect ratio", (done) => { ImageMock.defaultWidth = 140; ImageMock.defaultHeight = 140; grid.gridInstance.getImage = () => @@ -353,6 +353,40 @@ describe('Validate images', () => { } ); }); + it("fails if crops don't respect criteria without an aspect ration", (done) => { + ImageMock.defaultWidth = 140; + ImageMock.defaultHeight = 140; + grid.gridInstance.getImage = () => + Promise.resolve({ + data: { + exports: [ + { + id: 'image_crop', + assets: [ + { dimensions: { width: 900, height: 100 } }, + { dimensions: { width: 500, height: 10 } }, + { dimensions: { width: 50, height: 1 } }, + ], + }, + ], + }, + }); + + validateImageSrc( + 'http://grid.co.uk/1234567890123456789012345678901234567890', + 'front', + { + minWidth: 1000, + maxWidth: 2000, + } + ).then( + (err) => done.fail(err.toString()), + (err) => { + expect(err.message).toMatch(/does not have any valid crops/i); + done(); + } + ); + }); it('gets the first valid asset', (done) => { ImageMock.defaultWidth = 140; diff --git a/fronts-client/src/util/validateImageSrc.ts b/fronts-client/src/util/validateImageSrc.ts index 818a1a91e95..1672cdd2925 100644 --- a/fronts-client/src/util/validateImageSrc.ts +++ b/fronts-client/src/util/validateImageSrc.ts @@ -71,7 +71,10 @@ function getSuitableImageDetails( if (crops.length === 0) { return Promise.reject( new Error( - `The image does not have a valid ${widthAspectRatio}:${heightAspectRatio} crop on the Grid` + typeof widthAspectRatio === 'number' && + typeof heightAspectRatio === 'number' + ? `The image does not have a valid ${widthAspectRatio}:${heightAspectRatio} crop on the Grid` + : `The image does not have any valid crops on the Grid` ) ); } From 2b613879838a7802976b2f57224b1108fd33da04 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Wed, 12 Jun 2024 09:22:55 +0100 Subject: [PATCH 30/35] refactor validation to reduce duplication of dimension criteria checks --- fronts-client/src/util/validateImageSrc.ts | 41 +++++++--------------- 1 file changed, 13 insertions(+), 28 deletions(-) diff --git a/fronts-client/src/util/validateImageSrc.ts b/fronts-client/src/util/validateImageSrc.ts index 1672cdd2925..2af35c3e696 100644 --- a/fronts-client/src/util/validateImageSrc.ts +++ b/fronts-client/src/util/validateImageSrc.ts @@ -120,39 +120,21 @@ function validateActualImage(image: ImageDescription, frontId?: string) { const { width = 0, height = 0, - ratio = 0, + ratio, criteria, path, origin, thumb, } = image; - const { - maxWidth, - minWidth, - widthAspectRatio, - heightAspectRatio, - }: Criteria = criteria || {}; - const criteriaRatio = - widthAspectRatio && heightAspectRatio - ? widthAspectRatio / heightAspectRatio - : NaN; - if (maxWidth && maxWidth < width) { - return reject( - new Error(`Images cannot be more than ${maxWidth} pixels wide`) - ); - } else if (minWidth && minWidth > width) { - return reject( - new Error(`Images cannot be less than ${minWidth} pixels wide`) - ); - } else if (criteriaRatio && Math.abs(criteriaRatio - ratio) > 0.01) { - return reject( - new Error( - `Images must have a ${widthAspectRatio || ''}:${ - heightAspectRatio || '' - } aspect ratio` - ) + if (criteria) { + const dimensionValidation = validateDimensions( + { width, height, ratio }, + criteria ); + if (dimensionValidation.matchesCriteria === false) { + return reject(new Error(dimensionValidation.reason)); + } } if (image.origin) { return recordUsage(image.origin.split('/').slice(-1)[0], frontId).then( @@ -337,7 +319,7 @@ const getMaybeDimensionsFromWidthAndHeight = ( }; function validateDimensions( - dimensions: { width: number; height: number }, + dimensions: { width: number; height: number; ratio?: number }, criteria: Criteria ): { matchesCriteria: true } | { matchesCriteria: false; reason: string } { const { width, height } = dimensions; @@ -347,7 +329,10 @@ function validateDimensions( ? widthAspectRatio / heightAspectRatio : NaN; - const ratio = width / height; + // if validating a mediaItem with defined ratio value, use that instead of + // calculating from width and height + const ratio = + typeof dimensions.ratio == 'number' ? dimensions.ratio : width / height; if (maxWidth && maxWidth < width) { return { From bd1632b5ebb5eb9c17d269fee5e9d131a66e059f Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Wed, 12 Jun 2024 10:47:59 +0100 Subject: [PATCH 31/35] add a crop icon next to the Collection title if it uses 5:4 crops --- .../src/components/CollectionDisplay.tsx | 10 ++++++++ fronts-client/src/components/icons/Icons.tsx | 23 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/fronts-client/src/components/CollectionDisplay.tsx b/fronts-client/src/components/CollectionDisplay.tsx index 75c8905b386..165dff91e82 100644 --- a/fronts-client/src/components/CollectionDisplay.tsx +++ b/fronts-client/src/components/CollectionDisplay.tsx @@ -33,6 +33,8 @@ import { theme } from 'constants/theme'; import Button from 'components/inputs/ButtonDefault'; import { updateCollection as updateCollectionAction } from '../actions/Collections'; import { isMode } from '../selectors/pathSelectors'; +import { COLLECTIONS_USING_PORTRAIT_TRAILS } from 'constants/image'; +import { CropIcon, StarIcon } from './icons/Icons'; export const createCollectionId = ({ id }: Collection, frontId: string) => `front-${frontId}-collection-${id}`; @@ -254,6 +256,11 @@ class CollectionDisplay extends React.Component { const itemCount = articleIds ? articleIds.length : 0; const targetedTerritory = collection ? collection.targetedTerritory : null; const { displayName } = this.state; + + const usePortrait = + collection?.type && + COLLECTIONS_USING_PORTRAIT_TRAILS.includes(collection?.type); + return ( { > + {usePortrait && ( + + )} {this.state.editingContainerName ? ( ( ); +const CropIcon = ({ + fill = theme.colors.greyDark, + size = 'xl', + title = null, +}: IconProps) => ( + + {title} + + +); + export { DownCaretIcon, RubbishBinIcon, @@ -334,4 +356,5 @@ export { VideoIcon, DragHandleIcon as DragIcon, WarningIcon, + CropIcon, }; From e13642c9d678f404a0954077301589a367812c6d Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Wed, 12 Jun 2024 10:50:19 +0100 Subject: [PATCH 32/35] remove import --- fronts-client/src/components/CollectionDisplay.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fronts-client/src/components/CollectionDisplay.tsx b/fronts-client/src/components/CollectionDisplay.tsx index 165dff91e82..5c9dd57bcb0 100644 --- a/fronts-client/src/components/CollectionDisplay.tsx +++ b/fronts-client/src/components/CollectionDisplay.tsx @@ -34,7 +34,7 @@ import Button from 'components/inputs/ButtonDefault'; import { updateCollection as updateCollectionAction } from '../actions/Collections'; import { isMode } from '../selectors/pathSelectors'; import { COLLECTIONS_USING_PORTRAIT_TRAILS } from 'constants/image'; -import { CropIcon, StarIcon } from './icons/Icons'; +import { CropIcon } from './icons/Icons'; export const createCollectionId = ({ id }: Collection, frontId: string) => `front-${frontId}-collection-${id}`; From 14ee0c7258fde6106e97793418d5082ffdfb2977 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Wed, 12 Jun 2024 11:37:07 +0100 Subject: [PATCH 33/35] don't show the crop icon with feature switch off --- fronts-client/src/components/CollectionDisplay.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/fronts-client/src/components/CollectionDisplay.tsx b/fronts-client/src/components/CollectionDisplay.tsx index 5c9dd57bcb0..d659d179961 100644 --- a/fronts-client/src/components/CollectionDisplay.tsx +++ b/fronts-client/src/components/CollectionDisplay.tsx @@ -33,7 +33,10 @@ import { theme } from 'constants/theme'; import Button from 'components/inputs/ButtonDefault'; import { updateCollection as updateCollectionAction } from '../actions/Collections'; import { isMode } from '../selectors/pathSelectors'; -import { COLLECTIONS_USING_PORTRAIT_TRAILS } from 'constants/image'; +import { + COLLECTIONS_USING_PORTRAIT_TRAILS, + SUPPORT_PORTRAIT_CROPS, +} from 'constants/image'; import { CropIcon } from './icons/Icons'; export const createCollectionId = ({ id }: Collection, frontId: string) => @@ -258,6 +261,7 @@ class CollectionDisplay extends React.Component { const { displayName } = this.state; const usePortrait = + SUPPORT_PORTRAIT_CROPS && collection?.type && COLLECTIONS_USING_PORTRAIT_TRAILS.includes(collection?.type); From ddd0f30b792ff9d1bab731049b90cdff79f8f625 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Wed, 12 Jun 2024 11:47:18 +0100 Subject: [PATCH 34/35] remove test value from COLLECTIONS_USING_PORTRAIT_TRAILS --- fronts-client/src/constants/image.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fronts-client/src/constants/image.ts b/fronts-client/src/constants/image.ts index 5ff0a6e191a..54efad6a266 100644 --- a/fronts-client/src/constants/image.ts +++ b/fronts-client/src/constants/image.ts @@ -18,7 +18,7 @@ export const portraitCardImageCriteria = { }; // @todo - add the right collection type when it exists -export const COLLECTIONS_USING_PORTRAIT_TRAILS = ['fixed/small/slow-IV']; +export const COLLECTIONS_USING_PORTRAIT_TRAILS = []; export const defaultCardTrailImageCriteria = landScapeCardImageCriteria; From 377d48f461188d83d2d871e2bc9fa13edb5c7873 Mon Sep 17 00:00:00 2001 From: David Blatcher Date: Wed, 12 Jun 2024 11:53:45 +0100 Subject: [PATCH 35/35] define type for empty string array --- fronts-client/src/constants/image.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fronts-client/src/constants/image.ts b/fronts-client/src/constants/image.ts index 54efad6a266..447a83d034e 100644 --- a/fronts-client/src/constants/image.ts +++ b/fronts-client/src/constants/image.ts @@ -18,7 +18,7 @@ export const portraitCardImageCriteria = { }; // @todo - add the right collection type when it exists -export const COLLECTIONS_USING_PORTRAIT_TRAILS = []; +export const COLLECTIONS_USING_PORTRAIT_TRAILS: string[] = []; export const defaultCardTrailImageCriteria = landScapeCardImageCriteria;