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

dblatcher · 2024-06-10T15:28:55Z

What's changed?

Adds features (behind a feature switch) to the fronts tool to support some collection types using Portrait Crops for replacement trail images and/or slideshows

Utility code etc:

adds new feature switch "support-portrait-crops", hides it from the features menu of PROD
defines an empty string array COLLECTIONS_USING_PORTRAIT_TRAILS - in the "constants" folder. This can be populated with the collectionType of the new collection when implemented.
refactors image validation so the tests for whether a set of dimensions(as opposed to a whole media object) matches image Criteria can be exported separately (as below, this is used to validate if a crop dragged from one card match the criteria of the card it was dropped on
Criteria validation error strings (shown to the user in alerts) specific which type of crop is missing (eg "does not have a valid 5:4 crop" rather than "does not have any valid crops")

Component behaviour:
Card

adds collectionType prop (mapped from app State) and a determineCardCriteria using that prop
when a crop is dragged onto a Card from another Card, if will validate that other Card's Crop matches its criteria

ArticleBody & ThumbnailSmall

If there is a replacement image defined and the dimensions are portrait, ArticleBody passes a prop to ThumbnailSmall to vary the styling to suit a 4:5 image

ArticleFormMeta & InputImage

adds collectionType prop (mapped from app State) and a determineCardCriteria using that prop (like Card does) and proves the criteria to InputImage
refactors the styling code for the ImageContainer to allow for four variants (based on the values of the 'portrait' and 'small' boolean props.

CollectionDisplay

adds a 'crop' icon next to the title if the collection uses portrait crop (we should get design input before enabling the feature in earnest- not sure of the best way to communicate the difference in crop type)

testing locally

add a collection type (eg 'fixed/small/slow-IV') to COLLECTIONS_USING_PORTRAIT_TRAILS
open https://fronts.local.dev-gutools.co.uk/v2/features and set "Use portrait crops for the experimental big card containers" to "on"
open a front with a collection of of the type set to use portrait trail - you should see the "Crop" icon to the left of the title
Setting the feature switch to 'off' and refreshing the front page should remove the icon and stop you selecting portrait crops - however, if you have saved portrait crops on any of the cards, they will still be in place - if you try to replace them, the choice should be restricted to landscape crops.

Implementation notes

As long as COLLECTIONS_USING_PORTRAIT_TRAILS is empty, there should be no noticeable change in behaviour, other than the changes in the error alert including the crop dimensions of the criteria.

Using a hard coded string array on the frontend isn't the ideal way for configuring which collections use which crop type. It should be okay whilst this feature is experimental/in development.

The extra validation on drag and drop will be happening, but should:

never fail as all cards can only have 5:3 crops as replacement images
not impact performance. the image dimensions are looked up in the global state, not by querying the grid

Checklist

General

🤖 Relevant tests added
✅ CI checks / tests run locally
🔍 Checked on CODE

Client

🚫 No obvious console errors on the client (i.e. React dev mode errors)
🎛️ No regressions with existing user interactions (i.e. all existing buttons, inputs etc. work)
📷 Screenshots / GIFs of relevant UI changes included

Collection ("Reader's Tips") with Portrait crops:

Cards with portrait thumbnails:

…f the selected replacement image, use to style the ImageContainer

…collection-type

…ortraits in slideshows

…g crops'

…rops

…lay portraits right

…pect ratio

github-actions · 2024-06-11T16:54:17Z

Deploy build 8113 of `cms-fronts::facia-tool` to CODE

All deployment options

From guardian/actions-riff-raff.

rebecca-thompson

Have played with this locally and in CODE and it all works as expected. A couple of visual spacing issues, maybe?

Can we adjust the spacing, so the thumbnails are not overlapping?

It looks like this icon could use some right hand spacing. Also, just a question... is this the final icon or a placeholder? Seems an inconsistent choice given the crop icon generally represents re-cropping an image in our tools.

dblatcher · 2024-06-17T09:56:07Z

Have played with this locally and in CODE and it all works as expected. A couple of visual spacing issues, maybe?

Can we adjust the spacing, so the thumbnails are not overlapping?

Good point, I must have missed that. I'll take a look.

It looks like this icon could use some right hand spacing. Also, just a question... is this the final icon or a placeholder? Seems an inconsistent choice given the crop icon generally represents re-cropping an image in our tools.

It was intended as a placeholder, but you're right that its not ideal. I was thinking of it as a symbol for 'crops' in general, but that's not how it's used elsewhere. I think I'll swap the icon for a small badge component of some kind that can include the "5:4" text.

prout-bot · 2024-06-17T12:06:21Z

Seen on PROD (merged by @dblatcher 6 minutes and 23 seconds ago) Please check your changes!

…on drop (#1591) * use hard coded constant so all trails are required to be portrait * allow grid to select either image type, add state for the dimension of the selected replacement image, use to style the ImageContainer * bug fix? aspect ratio test should use absolute difference * use the criteria to set the gird url, not a separate prop * export the default trail image criteria from constants * remove unused imports * shorted var name, comments about use of constant criteria * add selectCollectionType selector * provide the collection type to the InputImage * add feature switch for whether to allow portrait crops * if the feature switch is enabled, set criteria based on collection type * dont change the FeatureSwitch class - just hide the new one client-side * provide criteria to slideshow * dont need state for imageDims - can get from props.input.value * remove collection type from render * use horrendous css funcions to style the image containers for small portraits in slideshows * wrangle the css for ImageContainer * move the image container to own file * validate image error distinguishes between 'no crops' and 'no matching crops' * dont use the default criteria * move constants to constants/image * Card components check the collection type to set criteria for image drops * add TO DO about dragging images from card to card * cards check replacement image dimensions and style thumbnails to display portraits right * validate dimensions of incoming dragged images from other cards * undo switch change * more coherent comments * update tests to account for more specific error messaging * message correctly when crops fail validation of a criteria without aspect ratio * refactor validation to reduce duplication of dimension criteria checks * add a crop icon next to the Collection title if it uses 5:4 crops * remove import * don't show the crop icon with feature switch off * remove test value from COLLECTIONS_USING_PORTRAIT_TRAILS * define type for empty string array

dblatcher added 30 commits June 4, 2024 18:10

use hard coded constant so all trails are required to be portrait

82c57d7

allow grid to select either image type, add state for the dimension o…

697be74

…f the selected replacement image, use to style the ImageContainer

bug fix? aspect ratio test should use absolute difference

f81efff

use the criteria to set the gird url, not a separate prop

cdfd2d6

export the default trail image criteria from constants

32be7d8

remove unused imports

3a1cb1b

shorted var name, comments about use of constant criteria

fa50870

add selectCollectionType selector

3663f8c

provide the collection type to the InputImage

7ff52d7

add feature switch for whether to allow portrait crops

4a930af

Merge branch 'dblatcher/allow-portrait-trails' into dblatcher/access-…

26525aa

…collection-type

if the feature switch is enabled, set criteria based on collection type

3f670a0

dont change the FeatureSwitch class - just hide the new one client-side

ed2ef42

provide criteria to slideshow

0431d36

dont need state for imageDims - can get from props.input.value

e18ff39

Merge branch 'dblatcher/allow-portrait-trails' into dblatcher/access-…

f4f1a5c

…collection-type

remove collection type from render

9408df2

use horrendous css funcions to style the image containers for small p…

4b4702f

…ortraits in slideshows

wrangle the css for ImageContainer

6e3ee6b

move the image container to own file

4299278

validate image error distinguishes between 'no crops' and 'no matchin…

e3e7df4

…g crops'

dont use the default criteria

558c4ab

move constants to constants/image

9807964

Card components check the collection type to set criteria for image d…

07a6d27

…rops

add TO DO about dragging images from card to card

3eb882c

cards check replacement image dimensions and style thumbnails to disp…

0f998a6

…lay portraits right

validate dimensions of incoming dragged images from other cards

57440c9

Merge branch 'main' into dblatcher/set-criteria-by-collection-type

8c7d347

undo switch change

936e9c6

more coherent comments

c3b52ab

dblatcher added 2 commits June 11, 2024 16:50

update tests to account for more specific error messaging

1f9c9ac

message correctly when crops fail validation of a criteria without as…

838694c

…pect ratio

dblatcher added 6 commits June 12, 2024 09:22

refactor validation to reduce duplication of dimension criteria checks

2b61387

add a crop icon next to the Collection title if it uses 5:4 crops

bd1632b

remove import

e13642c

don't show the crop icon with feature switch off

14ee0c7

remove test value from COLLECTIONS_USING_PORTRAIT_TRAILS

ddd0f30

define type for empty string array

377d48f

dblatcher marked this pull request as ready for review June 13, 2024 10:53

dblatcher requested a review from a team as a code owner June 13, 2024 10:53

rebecca-thompson approved these changes Jun 17, 2024

View reviewed changes

dblatcher merged commit f5a4d23 into main Jun 17, 2024
9 checks passed

dblatcher deleted the dblatcher/set-criteria-by-collection-type branch June 17, 2024 11:59

prout-bot added Pending-on-PROD Seen-on-PROD and removed Pending-on-PROD labels Jun 17, 2024

dblatcher mentioned this pull request Jun 17, 2024

improve portrait crop visuals #1598

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

dblatcher commented Jun 10, 2024 •

edited

Loading

github-actions bot commented Jun 11, 2024 •

edited

Loading

rebecca-thompson left a comment

dblatcher commented Jun 17, 2024

prout-bot commented Jun 17, 2024

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

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

Conversation

dblatcher commented Jun 10, 2024 • edited Loading

What's changed?

testing locally

Implementation notes

Checklist

General

Client

github-actions bot commented Jun 11, 2024 • edited Loading

Deploy build 8113 of cms-fronts::facia-tool to CODE

rebecca-thompson left a comment

Choose a reason for hiding this comment

dblatcher commented Jun 17, 2024

prout-bot commented Jun 17, 2024

dblatcher commented Jun 10, 2024 •

edited

Loading

github-actions bot commented Jun 11, 2024 •

edited

Loading

Deploy build 8113 of `cms-fronts::facia-tool` to CODE