Add overrides for the Chef card #1589

jonathonherbert · 2024-06-03T16:01:30Z

What's changed?

Adds the missing overrides for the Chef card – image and palette overrides. The palette data is copied from mis-en-place, the tool that Fronts will eventually replace, here.

Implementation notes

I've added the collection data in this PR, as it was a similar shape and it seemed to make sense to do the lot at once.
It'd be much nicer to have ADTs that reflected the fact that cards with different cardType values have different types of metadata. I've left that for now, as this PR was already large, but it'd be good to do this in another PR on both client and server.

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

github-actions · 2024-06-03T16:09:11Z

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

All deployment options

From guardian/actions-riff-raff.

fronts-client/src/components/form/PaletteForm.tsx

JustinPinner · 2024-06-03T17:24:19Z

fronts-client/src/constants/feastPalettes.ts

+ lightBackgroundHex: '#F9F9F5',
+ darkForegroundHex: '#F0C5B4',
+ darkBackgroundHex: '#363632',
+ imageURL: 'https://uploads.guim.co.uk/2024/03/21/Potatos.png',


Nitpick #1: Potatoes :)

ikr? not my images 😅

JustinPinner · 2024-06-03T17:24:25Z

fronts-client/src/constants/feastPalettes.ts

+ lightBackgroundHex: '#F9F9F5',
+ darkForegroundHex: '#ACD8E5',
+ darkBackgroundHex: '#363632',
+ imageURL: 'https://uploads.guim.co.uk/2024/03/21/TVsbacks.png',


Nitpick #2: TVsnacks?

as above, but we could always rename 'em, it's somewhat irritating

fronts-client/src/types/Collection.ts

fronts-client/src/components/form/ChefMetaForm.tsx

fronts-client/src/util/form.ts

fronts-client/src/actions/OptionsModal.ts

fredex42

Like it! I made some comments which I hope are helpful, though feel free to disregard as I don't know the frontend code on this project well

emdash-ie

I haven’t yet run this locally, but it looks good to me! I have some questions below, but I’m happy to merge as-is as well :)

fronts-client/src/components/form/ChefMetaForm.tsx

emdash-ie · 2024-06-12T14:38:50Z

fronts-client/src/components/form/ChefMetaForm.tsx


 return (
 <ConnectedChefForm
 chef={chef}
 chefWithoutOverrides={chefWithoutOverrides}
 card={card}
- initialValues={initialValues}
+ initialValues={card.meta as ChefCardMeta}


Similarly, a comment here explaining why this cast is ok might be helpful

👍 this should go away with a larger refactor: comment added in 8cb223e.

emdash-ie · 2024-06-12T14:43:48Z

fronts-client/src/components/form/PaletteForm.tsx

+ return undefined;
+ }
+
+ return formValues[fieldName] as Palette;


Is this as Palette needed, given the type for formValues above? I wouldn’t have expected it to be, but I have not checked!

Frustratingly, I think Object.entries will not give a union type for the object keys, even if they're known type – we just get string, requiring a cast further downstream.

In reminding myself why Typescript behaves this way, I stumbled across the first answer in this post on stackoverflow, which was very informative, and gave a few examples of a better-typed implementation! I've added that in 10213da. Thanks for the prompt, this is nicer.

Ah, good! Nice to have some clearer types. I don’t quite see how Object.entries is relevant here though: since the code declares the type of formValues to be {[T: string]: Palette;}, I’m quite surprised that the compiler can’t tell the type of formValues[fieldName] must be Palette. But also: why is the check if (!formValues) needed? (Maybe the type given for formValues should include undefined, and this is what’s confusing the compiler?)

It looks like you can remove as Palette without getting any complaints from the compiler. But I guess there’s no real need: it doesn’t provide any benefits, especially as I think the type of getFormValues(formName) is any => any.

I think I read your comment as relating to as PaletteId! The improvements above are fine, but spot on, this is not needed. Possibly artefact from previous model where keys were a union. 564fab7

emdash-ie · 2024-06-12T14:47:25Z

fronts-client/src/constants/feastPalettes.ts

+ darkBackgroundHex: '#363632',
+ imageURL: 'https://uploads.guim.co.uk/2024/03/21/Celebration.png',
+ },
+ 'Celebration\nCakes': {


A newline in a key?? I assume this is outside our control, but it sure is strange.

We're stuck for now, but I will point this out to Feast. It's weird.

…erride instead

Co-authored-by: Jonathon Herbert <jonathon.herbert@guardian.co.uk>

…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

prout-bot · 2024-06-20T09:48:03Z

Seen on PROD (merged by @jonathonherbert 9 minutes and 32 seconds ago) Please check your changes!

jonathonherbert requested a review from a team as a code owner June 3, 2024 16:01

JustinPinner reviewed Jun 3, 2024

View reviewed changes

fronts-client/src/components/form/PaletteForm.tsx Show resolved Hide resolved

JustinPinner reviewed Jun 3, 2024

View reviewed changes

Divs-B reviewed Jun 6, 2024

View reviewed changes

fronts-client/src/types/Collection.ts Show resolved Hide resolved

fredex42 reviewed Jun 6, 2024

View reviewed changes

fronts-client/src/components/form/ChefMetaForm.tsx Outdated Show resolved Hide resolved

fredex42 reviewed Jun 6, 2024

View reviewed changes

fronts-client/src/util/form.ts Show resolved Hide resolved

fredex42 reviewed Jun 6, 2024

View reviewed changes

fronts-client/src/actions/OptionsModal.ts Outdated Show resolved Hide resolved

fredex42 reviewed Jun 6, 2024

View reviewed changes

jonathonherbert force-pushed the jsh/chef-overrides branch 4 times, most recently from 0238025 to 53b15b5 Compare June 6, 2024 16:21

emdash-ie approved these changes Jun 12, 2024

View reviewed changes

jonathonherbert force-pushed the jsh/chef-overrides branch 2 times, most recently from 7ac4285 to 4a2a80b Compare June 13, 2024 20:20

jonathonherbert added 13 commits June 20, 2024 10:10

Add chef image field

7eb4c92

Move modals to own folder

4537b2e

Add modal for palette options

3fdf066

Add additional properties to card meta

adcdecd

Move palette components to own file for reuse

104b18e

Transform image overrides on way in and out to ensure correct meta

07ae0ef

Add custom palette picker

ea38d70

Use 'done' rather than 'cancel' for palette changes

329afb2

Lint

6625634

Wording for image field

833978b

Fix test cases

fe40766

Do not spread image overrides into existing fields, using chefImageOv…

8be01d7

…erride instead

Favour individual imports of lodash noop

8674eed

jonathonherbert and others added 9 commits June 20, 2024 10:12

Lint

19c1716

Separate type for palette

de003e3

Remove errant reference to ChefCardFormData

dfd9e61

Nest palette fields

8b5e23a

Explain cast in ChefMetaForm

e2d0563

Add better-typed entries fn for palette form

c9a3cb2

linting fix

2d915af

Co-authored-by: Jonathon Herbert <jonathon.herbert@guardian.co.uk>

Patch after rebase

1cf0783

jonathonherbert force-pushed the jsh/chef-overrides branch from 3e5fd60 to 1cf0783 Compare June 20, 2024 09:28

Remove unnecessary cast

564fab7

jonathonherbert merged commit 7764593 into main Jun 20, 2024
9 checks passed

jonathonherbert deleted the jsh/chef-overrides branch June 20, 2024 09:38

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

jonathonherbert mentioned this pull request Jul 30, 2024

Update Feast model to draw data from new card meta #1612

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add overrides for the Chef card #1589

Add overrides for the Chef card #1589

jonathonherbert commented Jun 3, 2024 •

edited

Loading

github-actions bot commented Jun 3, 2024 •

edited

Loading

JustinPinner Jun 3, 2024

jonathonherbert Jun 4, 2024

JustinPinner Jun 3, 2024

jonathonherbert Jun 4, 2024

fredex42 left a comment

emdash-ie left a comment

emdash-ie Jun 12, 2024

jonathonherbert Jun 13, 2024 •

edited

Loading

emdash-ie Jun 12, 2024

jonathonherbert Jun 13, 2024

emdash-ie Jun 14, 2024

emdash-ie Jun 14, 2024

jonathonherbert Jun 20, 2024

emdash-ie Jun 12, 2024

jonathonherbert Jun 13, 2024

prout-bot commented Jun 20, 2024

Add overrides for the Chef card #1589

Add overrides for the Chef card #1589

Conversation

jonathonherbert commented Jun 3, 2024 • edited Loading

What's changed?

Implementation notes

Checklist

General

Client

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fredex42 left a comment

Choose a reason for hiding this comment

emdash-ie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonathonherbert Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prout-bot commented Jun 20, 2024

jonathonherbert commented Jun 3, 2024 •

edited

Loading

github-actions bot commented Jun 3, 2024 •

edited

Loading

jonathonherbert Jun 13, 2024 •

edited

Loading