Fix prop type errors

[pjlee11]

Resolves #1688
Resolves #1690

Overall change: Fix all prop types warnings in the console

Code changes:

• Removed isRequired from alternateName propTypes
• Bumped the @bbc/psammead-caption version to get the correct prop definitions, also pulls in the new styling so there are visual changes as per Reduce caption & paragraph padding  BBC-archive/psammead#482 and Update caption BBC-archive/psammead#524
• Defines propTypes for the TimestampBlock
• Remove fixture data of superscript as it will not be implemented as a fragment going forward (details)
• Auto linting on the JSON fixtures I've edited

Test instructions

• npm run dev there should be no errors in the console relating to propTypes (There are some errors expected for the manifest because it 500's.)
• Caption padding bottom is expected to be reduced
• Any other visual regression should be raised as unexpected (EG: spacing and typography)

---

• I have assigned myself to this PR and the corresponding issues
• Tests added for new features
• Test engineer approval
• I have followed the merging checklist and this is ready to merge.

[pjlee11]

Since #1627 we now render <p> tags inside of the <figcaption> meaning that we are actually using <figcaption> as a wrapper to other HTML rather than rendering text contents inside of it. This was the correct solution because you can’t render multiple <figcaption>’s in a <figure>. The outcome of this is that the typography CSS is being ignored because both Caption defines typography and thenParagraph defines different typography styles.

I think there are a few solutions to this and wanted to know everyones opinion:

1. We make Paragraph typography agnostic and pass in the typography CSS as a prop (similar to how psammead-timestamp is currently working)
2. We utilise the styled-component’s as functionality and in simorgh have:

const CaptionParagraph = styled.p`
  // styles go here 
`;

  <Paragraph
    script={script}
    as={CaptionParagraph}
  />

3. Make script optional in psammead-paragraph so that the Caption’s CSS is used. by inheritance

PS: The current caption styling is only correct because we aren’t passing the required script which then causes a propType warning on npm run dev

[pjlee11]

Following on from the above:

Option 1 will work for all use cases but means we have to explicitly pass in a “typography” prop anywhere we render Paragraph. This then means for open source use of this component the application using it needs to pass a callback function to define the typography for the component. This feels like something that is very much a bespoke requirement for our application rather than for the component itself.

Option 2 means we are making a bespoke component in Simorgh, (or this could be in Psammead but I don’t think it has much use case to be split out), which means any override going forward means a bespoke component in Simorgh.

Option 3 could be a blueprint for creating a component which doesn’t explicitly need styling because it inherits from the parent (EG: , or

etc). This could cause bugs if the inheritance is misconfigured. If used right this would mean there is less CSS for the browser to compute but in larger teams bugs are more likely so the risk might outweigh the benefit

[sareh]

Thanks for that summary, Phil.
I think option 3 would work best, since we have psammead-caption@1.1.3 to include styles relating to the <p> and <i> tags nested within the <figcaption> element.
https://github.com/bbc/psammead/blob/d3f184ee554e32793feed3db65024ddb6db5c875/packages/components/psammead-caption/src/index.jsx#L39-L44

I've therefore suggested the changes to not pass in the script to the Paragraph component, so that the Caption typography styles can be retained.

[pjlee11]

Thinking this through I think we could have a combination of 1 and 3. Where the component accepts typography as a prop but it is optional. This means we have the following flexibility to explicitly set the components typography settings but also allow the component to be rendered with the typography inherited from the parent element.

So given the CaptionParagraph example, we could have the following:

import Caption as `psammead-caption`;
import Paragraph as `psammead-paragraph`;

const Figure = () => (
  <figure> 
    <Caption>
      <Paragraph> // Note that no script or typography value passed means inherit the typography from Caption
        ...
      </Paragraph>
    </Caption>
  </figure>
);

And in an example where we want to set the typography and not rely on inheritance:

import Caption as `psammead-caption`;
import Paragraph as `psammead-paragraph`;
import { latin } from '@bbc/gel-foundations/scripts';
import { getPica } from '@bbc/gel-foundations/typography';

const Figure = () => (
  <figure> 
    <Caption>
      <Paragraph    
        typographyFunc={getPica}
        script={latin}>
        ...
      </Paragraph>
    </Caption>
  </figure>
);

[sareh]

@pjlee11 Sounds good. Will you agree that the making of the script field optional can be done separately to this PR? So that we can have the case 1 that you've stated in this PR.

The second case shouldn't be needed for the caption - otherwise the typography styles for the caption will be overwritten by those for the paragraph.

(Also, there's also a fourth way to address this issue. We could change caption to using a <p> element instead of the paragraph from. We're currently doing this to handle <i> fragments within a caption & ensure that psammead-caption has the relevant styles. It'd be a simpler, cleaner approach, although there would be some duplication of styles in the page.)

For now, I'm happy to go with the option that you've stated above - a the mix of cases 1 & 3.

[sareh]

👍 Looks great. No more errors in the console. ✨

[pjlee11]

The following fixture data asset still have propTypes errors due:

Asset ID	Reason
c0000000003o	Contains video block which doesn't have an accurate propType definition
c0000000004o	Contains video block which doesn't have an accurate propType definition
c0000000005o	Contains video block which doesn't have an accurate propType definition
c0000000006o	Contains video block which doesn't have an accurate propType definition
c0000000009o	Contains video block which doesn't have an accurate propType definition
c0000000012o	Contains video block which doesn't have an accurate propType definition
c0000000013o	Contains video block which doesn't have an accurate propType definition
c0000000015o	Contains video block which doesn't have an accurate propType definition
c0000000016o	Contains video block which doesn't have an accurate propType definition
c0000000017o	Contains video block which doesn't have an accurate propType definition
c0000000018o	Contains video block which doesn't have an accurate propType definition
c0000000019o	Contains video block which doesn't have an accurate propType definition
c0000000022o	Contains video block which doesn't have an accurate propType definition

The video propType failures will be fixed within #1636

[sareh]

@pjlee11 Just to clarify - the proptypes for Video are accurate, the data validator/fixture files need to be updated to match: #766

Everything else looks good! Thanks for this.

[jamesbrumpton]

LGTM. Happy for this to be merged.