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.

Sign up for GitHub

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

Jump to bottom

Store recipes nested in feast collections, and ensure other card types are not permitted within them #1609

Merged
merged 7 commits into from
Aug 2, 2024
Merged

Store recipes nested in feast collections, and ensure other card types are not permitted within them #1609

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6445c70
Add subtype for supporting articles
jonathonherbert Jul 29, 2024
84db62c
'recipes' label for feast collection supporting cards
jonathonherbert Jul 29, 2024
1a33a08
'recipes' label for feast collection supporting cards
jonathonherbert Jul 29, 2024
b15d6b3
Discriminate by card type when permitting drop events, and alter mess…
jonathonherbert Jul 29, 2024
4885c5d
DRY up logic adding drop information when inserting cards
jonathonherbert Jul 29, 2024
19b6f9d
Clean up after reviewing the PR diff, factoring a few more common pro…
jonathonherbert Jul 29, 2024
88ec333
Inline drag event fn
jonathonherbert Aug 2, 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
12 changes: 10 additions & 2 deletions app/model/editions/EditionsCard.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,14 @@ object EditionsArticle extends Logging {
}
}

case class EditionsRecipe(id: String, addedOn: Long) extends EditionsCard {
// Only certain cards are permitted within a FeastCollection.
sealed trait EditionsFeastCollectionItem extends EditionsCard

object EditionsFeastCollectionItem {
implicit val format: OFormat[EditionsFeastCollectionItem] = Json.format[EditionsFeastCollectionItem]
}

case class EditionsRecipe(id: String, addedOn: Long) extends EditionsCard with EditionsFeastCollectionItem {
val cardType: CardType = CardType.Recipe
}

Expand Down Expand Up @@ -217,7 +224,8 @@ object EditionsChef {

case class EditionsFeastCollectionMetadata(
title: Option[String] = None,
theme: Option[FeastCollectionTheme] = None
theme: Option[FeastCollectionTheme] = None,
collectionItems: List[EditionsFeastCollectionItem] = List.empty
)

object EditionsFeastCollectionMetadata {
Expand Down
5 changes: 4 additions & 1 deletion app/model/editions/client/ClientCardMetadata.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ case class ClientCardMetadata(
chefImageOverride: Option[Image] = None, // Chef
title: Option[String] = None, // FeastCollection
feastCollectionTheme: Option[FeastCollectionTheme] = None, // FeastCollection
supporting: List[EditionsSupportingClientCard] = List.empty
) {
def toChefMetadata: EditionsChefMetadata =
EditionsChefMetadata(
Expand All @@ -54,6 +55,7 @@ case class ClientCardMetadata(
EditionsFeastCollectionMetadata(
title,
feastCollectionTheme,
collectionItems = supporting.map(EditionsSupportingClientCard.toFeastCollectionItem)
)

def toArticleMetadata: EditionsArticleMetadata = {
Expand Down Expand Up @@ -106,7 +108,8 @@ object ClientCardMetadata {
def fromCardMetadata(cardMetadata: EditionsFeastCollectionMetadata): ClientCardMetadata = {
ClientCardMetadata(
title = cardMetadata.title,
feastCollectionTheme = cardMetadata.theme
feastCollectionTheme = cardMetadata.theme,
supporting = cardMetadata.collectionItems.map(card => EditionsSupportingClientCard.fromFeastCollectionItem(card))
)
}

Expand Down
15 changes: 14 additions & 1 deletion app/model/editions/client/EditionsClientCollection.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import services.editions.prefills.CapiQueryTimeWindow
import model.editions.EditionsCard
import model.editions.EditionsArticle
import model.editions.{CapiPrefillQuery, EditionsCollection, EditionsRecipe, EditionsChef, EditionsFeastCollection, CardType}
import model.editions.EditionsFeastCollectionItem

// Ideally the frontend can be changed so we don't have this weird modelling!

case class EditionsClientCard(id: String, cardType: Option[CardType], frontPublicationDate: Long, meta: Option[ClientCardMetadata] = None)

object EditionsClientCard {
Expand Down Expand Up @@ -71,6 +71,19 @@ object EditionsClientCard {
}
}

case class EditionsSupportingClientCard(id: String, cardType: Option[CardType], frontPublicationDate: Long)

object EditionsSupportingClientCard {
implicit def format: OFormat[EditionsSupportingClientCard] = Json.format[EditionsSupportingClientCard]

def fromFeastCollectionItem(item: EditionsFeastCollectionItem) = item match {
case EditionsRecipe(id, addedOn) => EditionsSupportingClientCard(id, Some(CardType.Recipe), addedOn)
}

def toFeastCollectionItem(supportingCard: EditionsSupportingClientCard) =
EditionsRecipe(supportingCard.id, supportingCard.frontPublicationDate)
}

case class EditionsClientCollection(
id: String,
displayName: String,
Expand Down
13 changes: 13 additions & 0 deletions fronts-client/src/components/FrontsEdit/Collection.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { connect } from 'react-redux';
import type { State } from 'types/State';
import { createSelectArticleVisibilityDetails } from 'selectors/frontsSelectors';
import FocusWrapper from 'components/FocusWrapper';
import { CardTypes } from 'constants/cardTypes';

const getArticleNotifications = (
id: string,
Expand Down Expand Up @@ -184,6 +185,10 @@ class CollectionContext extends React.Component<ConnectedCollectionContextProps>
cardId={card.uuid}
onMove={handleMove}
onDrop={handleInsert}
cardTypeAllowList={this.getPermittedCardTypes(
card.cardType
)}
dropMessage={this.getDropMessage(card.cardType)}
>
{(supporting, getSupportingProps) => (
<Card
Expand Down Expand Up @@ -223,6 +228,14 @@ class CollectionContext extends React.Component<ConnectedCollectionContextProps>
</CollectionWrapper>
);
}

private getPermittedCardTypes = (
cardType?: CardTypes
): CardTypes[] | undefined =>
cardType === 'feast-collection' ? ['recipe'] : undefined;

private getDropMessage = (cardType?: CardTypes) =>
cardType === 'feast-collection' ? 'Place recipe here' : 'Sublink';
}

const createMapStateToProps = () => {
Expand Down
22 changes: 8 additions & 14 deletions fronts-client/src/components/FrontsEdit/CollectionComponents/DragToAddFeastCollection.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,14 @@
import React, { useRef } from 'react';
import v4 from 'uuid/v4';

import {
DraggingArticleComponent,
dragOffsetX,
dragOffsetY,
} from './ArticleDrag';
import { DraggingArticleComponent } from './ArticleDrag';
import { DragToAdd } from './DragToAdd';
import { Card } from 'types/Collection';
import { CardTypesMap } from 'constants/cardTypes';
import { handleDragStartForCard } from 'util/dragAndDrop';

const handleDragStart = (
event: React.DragEvent<HTMLDivElement>,
dragImageElement: HTMLDivElement | null
dragImageElement: HTMLDivElement
) => {
const feastCollectionCard: Card = {
cardType: CardTypesMap.FEAST_COLLECTION,
Expand All @@ -21,13 +17,11 @@ const handleDragStart = (
uuid: v4(),
frontPublicationDate: Date.now(),
};
event.dataTransfer.setData(

return handleDragStartForCard(
CardTypesMap.FEAST_COLLECTION,
JSON.stringify(feastCollectionCard)
);
if (dragImageElement) {
event.dataTransfer.setDragImage(dragImageElement, dragOffsetX, dragOffsetY);
}
feastCollectionCard
)(event, dragImageElement);
};

export const DragToAddFeastCollection = () => {
Expand All @@ -37,7 +31,7 @@ export const DragToAddFeastCollection = () => {
dragImage={<DraggingArticleComponent headline="Feast collection" />}
dragImageRef={ref}
onDragStart={(e: React.DragEvent<HTMLDivElement>) =>
handleDragStart(e, ref.current)
ref.current && handleDragStart(e, ref.current)
}
>
Drag to add a feast collection
Expand Down
11 changes: 6 additions & 5 deletions fronts-client/src/components/FrontsEdit/CollectionComponents/Sublinks.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ import CardContainer from 'components/card/CardContainer';
import CardContent from 'components/card/CardContent';
import CardMetaContainer from 'components/card/CardMetaContainer';
import DragIntentContainer from 'components/DragIntentContainer';
import { dragEventIsDenylisted } from 'lib/dnd/Level';
import { collectionDropTypeDenylist } from 'constants/fronts';
import { denyDragEvent } from 'lib/dnd/CardTypeLevel';

const SublinkCardBody = styled(CardBody)<{
dragHoverActive: boolean;
Expand Down Expand Up @@ -41,6 +40,8 @@ interface SublinkProps {
toggleShowArticleSublinks: (e?: React.MouseEvent) => void;
showArticleSublinks: boolean;
parentId: string;
// The singular label to show the user, e.g. 'sublink'.
sublinkLabel?: string;
}

class Sublinks extends React.Component<SublinkProps> {
Expand All @@ -54,6 +55,7 @@ class Sublinks extends React.Component<SublinkProps> {
toggleShowArticleSublinks,
showArticleSublinks,
parentId,
sublinkLabel = 'sublink',
} = this.props;

const isClipboard = parentId === 'clipboard';
Expand Down Expand Up @@ -82,7 +84,7 @@ class Sublinks extends React.Component<SublinkProps> {
{!isClipboard && <CardMetaContainer />}
<SublinkCardContent displaySize="small" showMeta={isClipboard}>
<span>
{numSupportingArticles} sublink
{numSupportingArticles} {sublinkLabel}
{numSupportingArticles > 1 && 's'}
<ButtonCircularCaret
openDir={showArticleSublinks ? 'up' : 'down'}
Expand All @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

export default Sublinks;
8 changes: 7 additions & 1 deletion fronts-client/src/components/card/Card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,13 @@ class Card extends React.Component<CardContainerProps> {
textSize={textSize}
showMeta={showMeta}
/>
{getSublinks}
<Sublinks
numSupportingArticles={numSupportingArticles}
toggleShowArticleSublinks={this.toggleShowArticleSublinks}
showArticleSublinks={this.state.showCardSublinks}
parentId={parentId}
sublinkLabel="recipe"
/>
{/* If there are no supporting articles, the children still need to be rendered, because the dropzone is a child */}
{numSupportingArticles === 0
? children
Expand Down
26 changes: 11 additions & 15 deletions fronts-client/src/components/clipboard/CardLevel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,25 @@
import React from 'react';
import { Level, LevelChild, MoveHandler, DropHandler } from 'lib/dnd';
import { LevelChild, MoveHandler, DropHandler } from 'lib/dnd';
import type { State } from 'types/State';
import { connect } from 'react-redux';
import { Card } from 'types/Collection';
import ArticleDrag, {
dragOffsetX,
dragOffsetY,
} from 'components/FrontsEdit/CollectionComponents/ArticleDrag';
import DropZone, {
DefaultDropContainer,
DefaultDropIndicator,
} from 'components/DropZone';
import { createSelectSupportingArticles } from 'selectors/shared';
import { collectionDropTypeDenylist } from 'constants/fronts';
import { theme, styled } from 'constants/theme';
import { CardTypeLevel } from 'lib/dnd/CardTypeLevel';
import { CardTypes } from 'constants/cardTypes';

interface OuterProps {
cardId: string;
children: LevelChild<Card>;
onMove: MoveHandler<Card>;
onDrop: DropHandler;
isUneditable?: boolean;
dropMessage?: string;
cardTypeAllowList?: CardTypes[];
}

interface InnerProps {
Expand Down Expand Up @@ -48,36 +47,33 @@ const CardLevel = ({
onMove,
onDrop,
isUneditable,
dropMessage,
cardTypeAllowList,
}: Props) => (
<Level
<CardTypeLevel
arr={supporting || []}
denylistedDataTransferTypes={collectionDropTypeDenylist}
parentType="card"
parentId={cardId}
type="card"
getId={({ uuid }) => uuid}
onMove={onMove}
onDrop={onDrop}
canDrop={!isUneditable}
renderDrag={(af) => <ArticleDrag id={af.uuid} />}
dragImageOffsetX={dragOffsetX}
dragImageOffsetY={dragOffsetY}
cardTypeAllowList={cardTypeAllowList}
renderDrop={
isUneditable
? undefined
: (props) => (
<DropZone
{...props}
dropColor={theme.base.colors.dropZoneActiveSublink}
dropMessage={'Sublink'}
dropMessage={dropMessage ?? 'Sublink'}
dropContainer={CardDropContainer}
dropIndicator={CardDropIndicator}
/>
)
}
>
{children}
</Level>
</CardTypeLevel>
);

const createMapStateToProps = () => {
Expand Down
18 changes: 4 additions & 14 deletions fronts-client/src/components/clipboard/ClipboardLevel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
import React from 'react';
import { Level, LevelChild, MoveHandler, DropHandler } from 'lib/dnd';
import { LevelChild, MoveHandler, DropHandler } from 'lib/dnd';
import type { State } from 'types/State';
import { selectClipboardArticles } from 'selectors/clipboardSelectors';
import { connect } from 'react-redux';
import { Card } from 'types/Collection';
import ArticleDrag, {
dragOffsetX,
dragOffsetY,
} from 'components/FrontsEdit/CollectionComponents/ArticleDrag';
import DropZone, { DefaultDropContainer } from 'components/DropZone';
import { collectionDropTypeDenylist } from 'constants/fronts';
import { styled, theme } from 'constants/theme';
import { CardTypeLevel } from 'lib/dnd/CardTypeLevel';

interface OuterProps {
children: LevelChild<Card>;
Expand Down Expand Up @@ -42,19 +38,13 @@ const ClipboardDropContainer = styled(DefaultDropContainer)<{
`;

const ClipboardLevel = ({ children, cards, onMove, onDrop }: Props) => (
<Level
<CardTypeLevel
containerElement={ClipboardItemContainer}
denylistedDataTransferTypes={collectionDropTypeDenylist}
arr={cards}
parentType="clipboard"
parentId="clipboard"
type="card"
dragImageOffsetX={dragOffsetX}
dragImageOffsetY={dragOffsetY}
getId={({ uuid }) => uuid}
onMove={onMove}
onDrop={onDrop}
renderDrag={(af) => <ArticleDrag id={af.uuid} />}
renderDrop={(props) => (
<DropZone
{...props}
Expand All @@ -64,7 +54,7 @@ const ClipboardLevel = ({ children, cards, onMove, onDrop }: Props) => (
)}
>
{children}
</Level>
</CardTypeLevel>
);

const mapStateToProps = (state: State) => ({
Expand Down
18 changes: 4 additions & 14 deletions fronts-client/src/components/clipboard/GroupLevel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,12 @@
import React from 'react';
import { Level, LevelChild, MoveHandler, DropHandler } from 'lib/dnd';
import { LevelChild, MoveHandler, DropHandler } from 'lib/dnd';
import type { State } from 'types/State';
import { connect } from 'react-redux';
import { Card } from 'types/Collection';
import ArticleDrag, {
dragOffsetX,
dragOffsetY,
} from 'components/FrontsEdit/CollectionComponents/ArticleDrag';
import DropZone, { DefaultDropContainer } from 'components/DropZone';
import { collectionDropTypeDenylist } from 'constants/fronts';
import { createSelectArticlesFromIds } from 'selectors/shared';
import { theme, styled } from 'constants/theme';
import { CardTypeLevel } from 'lib/dnd/CardTypeLevel';

interface OuterProps {
groupId: string;
Expand Down Expand Up @@ -65,19 +61,13 @@ const GroupLevel = ({
onDrop,
isUneditable,
}: Props) => (
<Level
<CardTypeLevel
arr={cards}
denylistedDataTransferTypes={collectionDropTypeDenylist}
parentType="group"
parentId={groupId}
type="card"
dragImageOffsetX={dragOffsetX}
dragImageOffsetY={dragOffsetY}
getId={({ uuid }) => uuid}
onMove={onMove}
onDrop={onDrop}
canDrop={!isUneditable}
renderDrag={(af) => <ArticleDrag id={af.uuid} />}
renderDrop={
isUneditable
? () => <Spacer />
Expand All @@ -92,7 +82,7 @@ const GroupLevel = ({
}
>
{children}
</Level>
</CardTypeLevel>
);

const createMapStateToProps = () => {
Expand Down
Loading
Loading