Improving display logic in the Card Footer

[cemms1]

The Card component is unwieldy. There is lots of nested logic related to rendering decisions and it's not clear which conditions result in what is rendered on the page.

One particularly difficult part of this is the "footer" and this concerns more than just the CardFooter component.

The card footer can contain any of the following:

• age
• comment count
• media pill
• branding
• "internal" sublinks
• "external" sublinks
• avatar cutout (for opinion cards)
  ...and possibly more!

We have various conditions for footer decisions including:

• isOpinionCardWithAvatar

  • dotcom-rendering/dotcom-rendering/src/components/Card/Card.tsx

    Lines 643 to 648 in 2ab2bdb

    	/**
    	* For opinion type cards with avatars (which aren't onwards content)
    	* we render the footer in a different location
    	*/
    	const isOpinionCardWithAvatar =
    	isOpinion && !isOnwardContent && media?.type === 'avatar';

• showPill

  • dotcom-rendering/dotcom-rendering/src/components/Card/Card.tsx

    Line 619 in 2ab2bdb

    const showPill = isMediaCardOrNewsletter && !isGallerySecondaryOnward;

• isStorylinesContent

  • dotcom-rendering/dotcom-rendering/src/components/Card/Card.tsx

    Lines 527 to 541 in 2ab2bdb

    	{/* Usually, we either display the pill or the footer,
    	but if the card appears in the storylines section on tag pages
    	then we do want to display the date on these cards as well as the media pill.
    	*/}
    	{isStorylines && (
    	<CardFooter
    	format={format}
    	age={decideAge()}
    	commentCount={<CommentCount />}
    	cardBranding={
    	isOnwardContent ? <LabsBranding /> : undefined
    	}
    	showLivePlayable={showLivePlayable}
    	/>
    	)}

• LabsBranding

It is very easy to create visual bugs when updating this area of code due to the lack of testing and clear logic to follow

The only testing we have for this component is Chromatic visual regression tests. This relies on the developer adding sufficient Storybook stories to cover each of the scenarios manually.

I think we should add unit tests for this logic to make it clearer for developers to understand and to catch any unintended consequences of updating code in this area.

We also need to put something in place to avoid returning to this scenario in the future (test coverage reporting? linting rules?)

How should we approach simplifying this logic?

[arelra]

I think it would help to break up Card into specialisations of 'Card', rather than a complex single component handling every possible variant.

If we have complex state derivation that is dependent on multiple pieces of state that may lend itself to useReducer .

The 'compound component' pattern might be useful to split up large components into sub components, allowing shared aspects (state or components) to be mixed-in.

https://www.patterns.dev/react/compound-pattern/
https://javascriptpatterns.vercel.app/patterns/react-patterns/compound-pattern

[abeddow91]

Similar to @arelra, I think Card would benefit hugely by separating out the distinct card types from one another (eg onwards, splash, newsletter, labs). Whilst they all share similar characteristics, they have distinct presentation rules and combining them into one complex component has led to a sea of exceptions.

The card component is also made by complicated by the fact that we are still supporting legacy containers. One immediate approach to simplifying card would be to remove support for legacy cards. This would also allow us to audit what is remaining more clearly.