Wsteama 1417 add related content link to jump to component

[LilyL0u]

Resolves JIRA [number]

Overall changes

A very high-level summary of easily-reproducible changes that can be understood by non-devs, and why these changes where made.

Code changes

• A bullet point list of key code changes that have been made.

Testing

1. List the steps used to test this PR.

Helpful Links

Add Links to useful resources related to this PR if applicable.

Coding Standards

Repository use guidelines

[amoore108]

#related-content-heading looks to be on the span, not the h2. This will likely have implications to focus state and possibly screen-readers, as we should be focusing on the h2 element.

[LilyL0u]

Oh I see you are referrring to the related content component itself?

Should we change this in this PR or in a different PR?

[amoore108]

Its maybe more of an accessibility question, I feel like we should be focusing on the h2 element but maybe its ok?

[LilyL0u]

We could raise it as a bug or improvement if @greenc05 thinks we should?

[LilyL0u]

@amoore108 I need to increase the max bundle size for the article page by 1kb apparently.
Should I do this by doing
export const MAX_SIZE = 1175 + 6;
or
export const MAX_SIZE = 1175 + 5;

or is there another way of preventing this?

[amoore108]

@amoore108 I need to increase the max bundle size for the article page by 1kb apparently. Should I do this by doing export const MAX_SIZE = 1175 + 6; or export const MAX_SIZE = 1175 + 5;

or is there another way of preventing this?

The output says:

Largest total bundle size (kB) (largest service + largest page)    │ 1181 |

Curious why its gone up with this change. There are quite a few diffs to the ArticlePage component though, are they all needed? Looks like linting and console logs.

The blocks component as well can be reverted back to its original state before this PR.

I'd make those changes before adjusting the bundle size to see if that helps.

[amoore108]

│ Smallest total bundle size (kB) (smallest service + smallest page) │ 643  │
├────────────────────────────────────────────────────────────────────┼──────┤
│ Largest total bundle size (kB) (largest service + largest page)    │ 1179 │
└────────────────────────────────────────────────────────────────────┴──────┘

This is from my PR, so I would change the MIN/MAX values in the bundleSizeConfig to the values from the build output in this PR. The +5 should stay as thats a buffer.

Its likely just slowly creeped up and is now getting caught by these min/max values.

[LilyL0u]

@amoore108 I need to increase the max bundle size for the article page by 1kb apparently. Should I do this by doing export const MAX_SIZE = 1175 + 6; or export const MAX_SIZE = 1175 + 5;
or is there another way of preventing this?

The output says:

Largest total bundle size (kB) (largest service + largest page)    │ 1181 |

Curious why its gone up with this change. There are quite a few diffs to the ArticlePage component though, are they all needed? Looks like linting and console logs.

The blocks component as well can be reverted back to its original state before this PR.

I'd make those changes before adjusting the bundle size to see if that helps.

I have removed the remaining dregs from Blocks and the console log from Article Page, but there is still the bundle size issue. In the comparison of the file in github it looks like loads of changes to the file, but almost all of it isn't actually a change and is just the code is on different lines due to adding

const showRelatedContent = blocks.some(
  block => block.type === 'relatedContent',
);

which shifts things down. Makes it hard to see the real differences, but there aren't actually many!

[LilyL0u]

I'm getting quite a bit of linting errors with the ArticlePage:

I don't see these in my vscode 🤔 Maybe the IDE isn't showing me linting errors properly? I'll format the document and see if that helps

[amoore108]

Worth deleting the blocks.find stuff and just having a separate story for the related content link?

Like:

 <Component
      jumpToHeadings={jumpToHeadings}
      showRelatedContentLink={false}
    />
...
 <Component
      jumpToHeadings={jumpToHeadings}
      showRelatedContentLink
    />

[amoore108]

Still a bit concerned about #12157 (comment), we're focusing on the span, not the related content h2 which I think will be impactful.

[amoore108]

Will this part of the condition ever be hit? Or was there a re-rendering thing happening whereby its was adding relatedContent to the array multiple times?

If thats happening, we should try not to mutate the original data, instead taking a copy of the jumpToHeadings and then pushing the relatedContent item to the copy, not the original.

[LilyL0u]

Yes it was adding the link again each rerender. I have been working on trying the copy of the array in storybook as it happens there too. I will do the same in this code too when I have got it working 👍

[amoore108]

Yea it may be good to try and construct an array of 'things to render', including the related content if applicable, so something like:

  const headingsToRender = [
    // Article subheadings
    ...jumpToHeadings.map(({ heading }) => ({
      heading,
      id: idSanitiser(heading),
    })),
    // Related content
    showRelatedContentLink && {
      heading: relatedContent,
      id: 'section-label-heading-related-content-heading',
    },
  ];

That'll take a shallow copy of the jumpToHeadings, so shouldn't cause the original data to be mutated and keep appending items to it on re-renders.

Changes implemented