Store recipes nested in feast collections, and ensure other card types are not permitted within them #1609
Conversation
which sadly cannot be modelled recursively, as the DynamoDB serialisers blow up – instead, we model a distinct type on the client, EditionsSupportingClientCard, which omits metadata altogether (for now), as no supporting cards require it. When metadata is eventually required, we will need a separate type for it (not EditionsClientMetadata), as it must not include a property that allows arbitrarily nested cards
…aging to match card type
jonathonherbert
force-pushed
the
jsh/permit-supporting-recipes
branch
from
6f23d16
to
b15d6b3
Compare
| // behaviour to be bypassed. | ||
| denylistedDataTransferTypes?: string[]; | ||
| // If this function returns false, all dragging behaviour is bypassed. | ||
| denyDragEvent?: (e: React.DragEvent) => boolean; |
There was a problem hiding this comment.
This is now a function because we no longer what consumers will have to match on until render time.
| }), | ||
| text: (url: string): RefDrop => ({ type: 'REF', data: url }), | ||
| }; | ||
| const dropToCardMap = { |
There was a problem hiding this comment.
Moved to its own data structure so we could statically derive its keys, and get some type safety upstream.
| import { CARD_TYPE } from 'lib/dnd/constants'; | ||
| import { InsertDropType } from './collectionUtils'; | ||
|
|
||
| export const handleDragStartForCard = |
There was a problem hiding this comment.
Helper, as this code was repeated in four different places.
…ps into CardTypeLevel
| @@ -98,8 +100,7 @@ class Sublinks extends React.Component<SublinkProps> { | |||
| ); | |||
| } | |||
|
|
|||
| private dragEventNotDenylisted = (e: React.DragEvent) => | |||
| !dragEventIsDenylisted(e, collectionDropTypeDenylist); | |||
| private dragEventNotDenylisted = (e: React.DragEvent) => !denyDragEvent()(e); | |||
There was a problem hiding this comment.
Not sure I understand what this is for - is in why the double-negative, and whether it would be clearer to remove the indirection through the method given it's just a single call?
There was a problem hiding this comment.
Yes, I see no reason not to move to an inline method! The reflex to use a method may have been related to a common pattern when class components were used and arrow fns were useful for lexical scoping, but that doesn't apply here. 88ec333
|
Overdue on PROD (merged by @jonathonherbert 30 minutes and 3 seconds ago) What's gone wrong? |
|
Seen on PROD (merged by @jonathonherbert 2 hours, 55 minutes and 57 seconds ago) Please check your changes! |
What's changed?
Stores recipes nested in feast collections, and ensures that other card types are not permitted within them.
Note the refresh, indicating that the data is stored correctly. Prior to this PR, refreshing the page would remove the nested items.
Implementation notes
Although it's no longer possible to put chefs and feast collections within feast collections, the drop icons do still appear when the user hovers over them. I timeboxed fixing this but it turned out to be a bit more complicated that expected! Perhaps we can circle back if user feedback suggests it's a priority.
It wasn't far off the beaten track for this PR, so I've added some messaging changes, too – the drop placeholders say 'Place recipe here' (they used to say 'sublink'), and the sublink expander at the bottom of the Feast Collection card says 'recipes' rather than 'sublinks'.
How to test
All tested locally.
Checklist
General
Client