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

Restrict splash to one story #1753

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
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
6 changes: 5 additions & 1 deletion fronts-client/src/components/FrontsEdit/Collection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ class CollectionContext extends React.Component<ConnectedCollectionContextProps>
canPublish={browsingStage !== 'live'}
browsingStage={browsingStage}
>
{(group, isUneditable, showGroupName) => (
{(group, isUneditable, groupIds, showGroupName) => (
<div key={group.uuid}>
<GroupDisplayComponent
key={group.uuid}
Expand All @@ -160,6 +160,8 @@ class CollectionContext extends React.Component<ConnectedCollectionContextProps>
<GroupLevel
isUneditable={isUneditable}
groupId={group.uuid}
groupName={group.name ? group.name : ''}
groupIds={groupIds}
onMove={handleMove}
onDrop={handleInsert}
cardIds={group.cards}
Expand Down Expand Up @@ -193,6 +195,8 @@ class CollectionContext extends React.Component<ConnectedCollectionContextProps>
<CardLevel
isUneditable={isUneditable}
cardId={card.uuid}
groupName={group.name ? group.name : ''}
groupIds={groupIds}
onMove={handleMove}
onDrop={handleInsert}
cardTypeAllowList={this.getPermittedCardTypes(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ interface CollectionPropsBeforeState {
children: (
group: Group,
isUneditable: boolean,
groupIds: string[],
showGroupName?: boolean,
) => React.ReactNode;
alsoOn: { [id: string]: AlsoOnDetail };
Expand Down Expand Up @@ -223,6 +224,8 @@ class Collection extends React.Component<CollectionProps, CollectionState> {

const isUneditable = isCollectionLocked || browsingStage !== cardSets.draft;

const groupIds = groups.map((group) => group.uuid);

return (
<>
<CollectionDisplay
Expand Down Expand Up @@ -338,7 +341,7 @@ class Collection extends React.Component<CollectionProps, CollectionState> {
) : null
}
>
{groups.map((group) => children(group, isUneditable, true))}
{groups.map((group) => children(group, isUneditable, groupIds, true))}
{hasContent && (
<EditModeVisibility visibleMode="fronts">
<PreviouslyCollectionContainer data-testid="previously">
Expand All @@ -357,7 +360,7 @@ class Collection extends React.Component<CollectionProps, CollectionState> {
launched they will not appear here.
</PreviouslyCollectionInfo>
<PreviouslyGroupsWrapper>
{children(previousGroup, true, false)}
{children(previousGroup, true, groupIds, false)}
</PreviouslyGroupsWrapper>
</>
)}
Expand Down
154 changes: 150 additions & 4 deletions fronts-client/src/components/FrontsEdit/FrontContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,159 @@ class FrontContent extends React.Component<FrontProps, FrontState> {
}

public handleMove = (move: Move<TCard>) => {
events.dropArticle(this.props.id, 'collection');
this.props.moveCard(move.to, move.data, move.from || null, 'collection');
const numberOfArticlesAlreadyInGroup = move.to.cards?.length;

// if we are inserting an article into any group that is not the splash, then we just insert
if (move.to.groupName !== 'splash') {
events.dropArticle(this.props.id, 'collection');
this.props.moveCard(move.to, move.data, move.from || null, 'collection');
} else {
// if we're in the splash and we insert an article and there's no other article already in the splash, then we just insert
if (numberOfArticlesAlreadyInGroup === 0 || undefined) {
events.dropArticle(this.props.id, 'collection');
this.props.moveCard(
move.to,
move.data,
move.from || null,
'collection',
);
}
// if we're in the splash and we insert an article and there's already another article, then we also look at the index we're inserting to
// if we're inserting to index 0, i.e. top of the group, then we want to grab the pre-existing article and move it to the other group
else if (
!!move.to.groupIds &&
move.to.cards !== undefined &&
move.to.index === 0
) {
//we do the regular move steps for the article we're moving to splash
events.dropArticle(this.props.id, 'collection');
this.props.moveCard(
move.to,
move.data,
move.from || null,
'collection',
);

//then we need to move the other article to the other group
const otherGroup = move.to.groupIds.filter(
(groupId) => groupId !== move.to.id,
)[0];
const existingCardData = move.to.cards[0];
const existingCardTo = {
index: 0,
id: otherGroup,
type: 'group',
groupIds: move.to.groupIds,
};
const existingCardMoveData: Move<TCard> = {
data: existingCardData,
from: false,
to: existingCardTo,
};
this.handleMove(existingCardMoveData);
}

// if we're in the splash and we insert an article and there's already another article, then we also look at the index we're inserting to
// if we're inserting to index 1, i.e. bottom of the group, then we add this story to the other group
else if (
!!move.to.groupIds &&
!!numberOfArticlesAlreadyInGroup &&
numberOfArticlesAlreadyInGroup > 0 &&
move.to.index > 0
) {
const otherGroup = move.to.groupIds.filter(
(groupId) => groupId !== move.to.id,
)[0];

const amendedTo = {
index: 0,
id: otherGroup,
type: 'group',
groupIds: move.to.groupIds,
};
events.dropArticle(this.props.id, 'collection');

this.props.moveCard(
amendedTo,
move.data,
move.from || null,
'collection',
);
}
}
};

public handleInsert = (e: React.DragEvent, to: PosSpec) => {
events.dropArticle(this.props.id, isDropFromCAPIFeed(e) ? 'feed' : 'url');
this.props.insertCardFromDropEvent(e, to, 'collection');
const numberOfArticlesAlreadyInGroup = to.cards?.length;

// if we are inserting an article into any group that is not the splash, then we just insert
if (to.groupName !== 'splash') {
events.dropArticle(this.props.id, isDropFromCAPIFeed(e) ? 'feed' : 'url');
this.props.insertCardFromDropEvent(e, to, 'collection');
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

I find the nested if else quite difficult to parse, what would you think about refactoring this code to use early returns instead? I think it might make this decision logic easier to follow.

// if we're in the splash and we insert an article and there's no other article already in the splash, then we just insert
if (numberOfArticlesAlreadyInGroup === 0 || undefined) {
events.dropArticle(
this.props.id,
isDropFromCAPIFeed(e) ? 'feed' : 'url',
Copy link
Contributor

Choose a reason for hiding this comment

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

The isDropFromCAPIFeed(e) ? 'feed' : 'url' gets used several times, could we refactor this out to line 266 and store in a variable?

);
this.props.insertCardFromDropEvent(e, to, 'collection');
}
// if we're in the splash and we insert an article and there's already another article, then we also look at the index we're inserting to
// if we're inserting to index 0, i.e. top of the group, then we want to grab the pre-existing article and move it to the other group
else if (!!to.groupIds && to.cards !== undefined && to.index === 0) {
// we do the regular insert steps for the article we're inserting to splash
events.dropArticle(
this.props.id,
isDropFromCAPIFeed(e) ? 'feed' : 'url',
);
this.props.insertCardFromDropEvent(e, to, 'collection');

//then we need to move the other article to the other group

const otherGroup = to.groupIds.filter(
(groupId) => groupId !== to.id,
)[0];
const existingCardData = to.cards[0];
const existingCardTo = {
index: 0,
id: otherGroup,
type: 'group',
groupIds: to.groupIds,
};
const existingCardMoveData: Move<TCard> = {
data: existingCardData,
from: false,
to: existingCardTo,
};
this.handleMove(existingCardMoveData);
}

// if we're in the splash and we insert an article and there's already another article, then we also look at the index we're inserting to
// if we're inserting to index 1, i.e. bottom of the group, then we add this story to the other group
else if (
!!to.groupIds &&
!!numberOfArticlesAlreadyInGroup &&
numberOfArticlesAlreadyInGroup > 0 &&
to.index > 0
) {
const otherGroup = to.groupIds.filter(
(groupId) => groupId !== to.id,
)[0];

const amendedTo = {
index: 0,
id: otherGroup,
type: 'group',
groupIds: to.groupIds,
};
events.dropArticle(
this.props.id,
isDropFromCAPIFeed(e) ? 'feed' : 'url',
);
this.props.insertCardFromDropEvent(e, amendedTo, 'collection');
}
}
};

public render() {
Expand Down
6 changes: 6 additions & 0 deletions fronts-client/src/components/clipboard/CardLevel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ interface OuterProps {
isUneditable?: boolean;
dropMessage?: string;
cardTypeAllowList?: CardTypes[];
groupName?: string;
groupIds?: string[];
}

interface InnerProps {
Expand Down Expand Up @@ -49,11 +51,15 @@ const CardLevel = ({
isUneditable,
dropMessage,
cardTypeAllowList,
groupName,
groupIds,
}: Props) => (
<CardTypeLevel
arr={supporting || []}
parentType="card"
parentId={cardId}
groupName={groupName}
groupIds={groupIds}
onMove={onMove}
onDrop={onDrop}
canDrop={!isUneditable}
Expand Down
6 changes: 6 additions & 0 deletions fronts-client/src/components/clipboard/GroupLevel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ interface OuterProps {
onMove: MoveHandler<Card>;
onDrop: DropHandler;
isUneditable?: boolean;
groupName: string;
groupIds: string[];
}

interface InnerProps {
Expand Down Expand Up @@ -60,11 +62,15 @@ const GroupLevel = ({
onMove,
onDrop,
isUneditable,
groupName,
groupIds,
}: Props) => (
<CardTypeLevel
arr={cards}
parentType="group"
parentId={groupId}
groupName={groupName}
groupIds={groupIds}
onMove={onMove}
onDrop={onDrop}
canDrop={!isUneditable}
Expand Down
8 changes: 8 additions & 0 deletions fronts-client/src/lib/dnd/Level.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ interface PosSpec {
type: string;
id: string;
index: number;
groupName?: string;
groupIds?: string[];
cards?: any[];
}

const isOnSameLevel = (from: PosSpec, to: PosSpec): boolean =>
Expand Down Expand Up @@ -56,6 +59,8 @@ export interface LevelProps<T> {
children: LevelChild<T>;
parentId: string;
parentType: string;
groupName?: string;
groupIds?: string[];
type: string;
getDropType?: (item: T) => string;
dragImageOffsetX?: number;
Expand Down Expand Up @@ -178,6 +183,9 @@ class Level<T> extends React.Component<Props<T>, State> {
index: i,
type: this.props.parentType,
id: this.props.parentId,
groupName: this.props.groupName,
groupIds: this.props.groupIds,
cards: this.props.arr,
};

if (!af) {
Expand Down
27 changes: 24 additions & 3 deletions fronts-client/src/lib/dnd/__tests__/dnd.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,14 @@ describe('Curation', () => {
expect(edit).toEqual({
data: { id: '1' },
from: { type: 'b', id: '0', index: 0 },
to: { type: 'a', id: '2', index: 1 },
to: {
cards: [{ id: '3' }, { id: '4' }],
groupName: undefined,
groupIds: undefined,
id: '2',
index: 1,
type: 'a',
},
});
});

Expand Down Expand Up @@ -164,7 +171,14 @@ describe('Curation', () => {
expect(edit).toEqual({
data: { id: '1' },
from: { type: 'b', id: '0', index: 0 },
to: { type: 'a', id: '2', index: 0 },
to: {
cards: [{ id: '3' }, { id: '4' }],
groupName: undefined,
groupIds: undefined,
id: '2',
index: 0,
type: 'a',
},
});

runDrag(nodeProps)(dropProps, false);
Expand Down Expand Up @@ -273,7 +287,14 @@ describe('Curation', () => {
})(dropProps);

expect(JSON.parse(event.dataTransfer.getData('text'))).toEqual(data);
expect(to).toEqual({ id: '2', index: 1, type: 'a' });
expect(to).toEqual({
cards: [{ id: '1' }],
groupName: undefined,
groupIds: undefined,
id: '2',
index: 1,
type: 'a',
});
});

it('does not allow moves of a node to a subPath of that node', () => {
Expand Down