Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

set criteria by collection type, style portrait thumbnails, validate on drop #1591

Merged
merged 38 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
82c57d7
use hard coded constant so all trails are required to be portrait
dblatcher Jun 4, 2024
697be74
allow grid to select either image type, add state for the dimension o…
dblatcher Jun 5, 2024
f81efff
bug fix? aspect ratio test should use absolute difference
dblatcher Jun 5, 2024
cdfd2d6
use the criteria to set the gird url, not a separate prop
dblatcher Jun 5, 2024
32be7d8
export the default trail image criteria from constants
dblatcher Jun 5, 2024
3a1cb1b
remove unused imports
dblatcher Jun 5, 2024
fa50870
shorted var name, comments about use of constant criteria
dblatcher Jun 5, 2024
3663f8c
add selectCollectionType selector
dblatcher Jun 6, 2024
7ff52d7
provide the collection type to the InputImage
dblatcher Jun 6, 2024
4a930af
add feature switch for whether to allow portrait crops
dblatcher Jun 6, 2024
26525aa
Merge branch 'dblatcher/allow-portrait-trails' into dblatcher/access-…
dblatcher Jun 6, 2024
3f670a0
if the feature switch is enabled, set criteria based on collection type
dblatcher Jun 6, 2024
ed2ef42
dont change the FeatureSwitch class - just hide the new one client-side
dblatcher Jun 7, 2024
0431d36
provide criteria to slideshow
dblatcher Jun 7, 2024
e18ff39
dont need state for imageDims - can get from props.input.value
dblatcher Jun 7, 2024
f4f1a5c
Merge branch 'dblatcher/allow-portrait-trails' into dblatcher/access-…
dblatcher Jun 7, 2024
9408df2
remove collection type from render
dblatcher Jun 7, 2024
4b4702f
use horrendous css funcions to style the image containers for small p…
dblatcher Jun 7, 2024
6e3ee6b
wrangle the css for ImageContainer
dblatcher Jun 7, 2024
4299278
move the image container to own file
dblatcher Jun 7, 2024
e3e7df4
validate image error distinguishes between 'no crops' and 'no matchin…
dblatcher Jun 7, 2024
558c4ab
dont use the default criteria
dblatcher Jun 10, 2024
9807964
move constants to constants/image
dblatcher Jun 10, 2024
07a6d27
Card components check the collection type to set criteria for image d…
dblatcher Jun 10, 2024
3eb882c
add TO DO about dragging images from card to card
dblatcher Jun 10, 2024
0f998a6
cards check replacement image dimensions and style thumbnails to disp…
dblatcher Jun 10, 2024
57440c9
validate dimensions of incoming dragged images from other cards
dblatcher Jun 10, 2024
8c7d347
Merge branch 'main' into dblatcher/set-criteria-by-collection-type
dblatcher Jun 10, 2024
936e9c6
undo switch change
dblatcher Jun 10, 2024
c3b52ab
more coherent comments
dblatcher Jun 10, 2024
1f9c9ac
update tests to account for more specific error messaging
dblatcher Jun 11, 2024
838694c
message correctly when crops fail validation of a criteria without as…
dblatcher Jun 11, 2024
2b61387
refactor validation to reduce duplication of dimension criteria checks
dblatcher Jun 12, 2024
bd1632b
add a crop icon next to the Collection title if it uses 5:4 crops
dblatcher Jun 12, 2024
e13642c
remove import
dblatcher Jun 12, 2024
14ee0c7
don't show the crop icon with feature switch off
dblatcher Jun 12, 2024
ddd0f30
remove test value from COLLECTIONS_USING_PORTRAIT_TRAILS
dblatcher Jun 12, 2024
377d48f
define type for empty string array
dblatcher Jun 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion app/model/FeatureSwitches.scala
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,14 @@ object TenImageSlideshows extends FeatureSwitch(
enabled = false
)

object UsePortraitCropsForSomeCollectionTypes extends FeatureSwitch(
key = "support-portrait-crops",
title = "Use portrait crops for the experimental big card containers",
enabled = false
)

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 {
Expand Down
14 changes: 14 additions & 0 deletions fronts-client/src/components/CollectionDisplay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ 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,
SUPPORT_PORTRAIT_CROPS,
} from 'constants/image';
import { CropIcon } from './icons/Icons';

export const createCollectionId = ({ id }: Collection, frontId: string) =>
`front-${frontId}-collection-${id}`;
Expand Down Expand Up @@ -254,6 +259,12 @@ class CollectionDisplay extends React.Component<Props, CollectionState> {
const itemCount = articleIds ? articleIds.length : 0;
const targetedTerritory = collection ? collection.targetedTerritory : null;
const { displayName } = this.state;

const usePortrait =
SUPPORT_PORTRAIT_CROPS &&
collection?.type &&
COLLECTIONS_USING_PORTRAIT_TRAILS.includes(collection?.type);

return (
<CollectionContainer
id={collection && createCollectionId(collection, frontId)}
Expand All @@ -264,6 +275,9 @@ class CollectionDisplay extends React.Component<Props, CollectionState> {
>
<CollectionHeadingSticky tabIndex={-1}>
<CollectionHeadingInner>
{usePortrait && (
<CropIcon title={'uses portrait (5:4) image crops'} />
)}
<CollectionHeadlineWithConfigContainer>
{this.state.editingContainerName ? (
<CollectionHeaderInput
Expand Down
15 changes: 14 additions & 1 deletion fronts-client/src/components/Features/FeaturesForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -13,12 +14,24 @@ interface Props {
setFeatureValue: (featureSwitch: FeatureSwitch) => void;
}

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 =>
STAGE === 'code' || !SWITCHES_TO_HIDE_ON_PROD.includes(featureSwitch.key);

class FeaturesForm extends React.Component<Props> {
public render() {
const { featureSwitches } = this.props;
console.log(featureSwitches);
return (
<form>
{featureSwitches.map((featureSwitch) => (
{featureSwitches.filter(filterSwitchesByStage).map((featureSwitch) => (
<InputCheckboxToggle
key={featureSwitch.key}
label={featureSwitch.title}
Expand Down
86 changes: 80 additions & 6 deletions fronts-client/src/components/card/Card.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -17,12 +18,18 @@ import {
} from 'actions/Cards';
import {
dragEventHasImageData,
getMaybeDimensionsFromWidthAndHeight,
validateDimensions,
validateImageEvent,
ValidationResponse,
} from 'util/validateImageSrc';
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';
Expand All @@ -46,6 +53,9 @@ 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';
import { Card as CardType } from 'types/Collection';

export const createCardId = (id: string) => `collection-item-${id}`;

Expand Down Expand Up @@ -104,6 +114,8 @@ type CardContainerProps = ContainerProps & {
editMode: EditMode;
isLive?: boolean;
pillarId?: string;
collectionType?: string;
selectOtherCard: { (uuid: string): CardType };
};

class Card extends React.Component<CardContainerProps> {
Expand Down Expand Up @@ -336,30 +348,90 @@ class Card extends React.Component<CardContainerProps> {
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) {
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 of 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
: defaultCardTrailImageCriteria;

// Our drag contains Grid data
validateImageEvent(e, this.props.frontId, imageCriteria)
.then((imageData) =>
this.props.addImageToCard(this.props.uuid, imageData)
)
.catch(alert);
};

private determineCardCriteria = (): Criteria => {
const { collectionType, parentId } = this.props;
// @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;
}

if (!SUPPORT_PORTRAIT_CROPS || !collectionType) {
return landScapeCardImageCriteria;
}

return COLLECTIONS_USING_PORTRAIT_TRAILS.includes(collectionType)
? portraitCardImageCriteria
: landScapeCardImageCriteria;
};

private checkDraggedImage = (
cardUuid: string,
imageCriteria: Criteria
): ReturnType<typeof validateDimensions> => {
// 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 = () => {
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),
Expand All @@ -368,6 +440,8 @@ const createMapStateToProps = () => {
pillarId: maybeExternalArticle && maybeExternalArticle.pillarId,
numSupportingArticles: selectSupportingArticleCount(state, uuid),
editMode: selectEditMode(state),
collectionType: collectionId && selectCollectionType(state, collectionId),
selectOtherCard: (uuid: string) => selectCard(state, uuid),
};
};
};
Expand Down
15 changes: 15 additions & 0 deletions fronts-client/src/components/card/article/ArticleBody.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -131,6 +132,8 @@ interface ArticleBodyProps {
canShowPageViewData: boolean;
frontId: string;
collectionId?: string;
imageSrcWidth?: string;
imageSrcHeight?: string;
}

const articleBodyDefault = React.memo(
Expand Down Expand Up @@ -176,11 +179,22 @@ 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 = getMaybeDimensionsFromWidthAndHeight(
imageSrcWidth,
imageSrcHeight
);
const thumbnailIsPortrait =
!!imageReplace &&
thumbnailDims &&
thumbnailDims.height > thumbnailDims.width;

return (
<>
{showMeta && (
Expand Down Expand Up @@ -294,6 +308,7 @@ const articleBodyDefault = React.memo(
imageHide={imageHide}
url={thumbnail}
isDraggingImageOver={isDraggingImageOver}
isPortrait={thumbnailIsPortrait}
>
{cutoutThumbnail ? (
<ThumbnailCutout src={cutoutThumbnail} />
Expand Down
Loading
Loading