Remove ArticleDesign.PrintShop

[JamieB-gu]

We no longer intend to handle this as a separate ArticleDesign. Anything that was previously PrintShop will now be one of the other ArticleDesigns. This change will require updates in DCAR and AR.

This is a breaking change because it removes this member from the enum. Therefore any code the depends on this member will need to be updated.

For example, in a switch the case will need to be removed:

switch (design) {
    case ArticleDesign.Standard:
      // Other code
    case ArticleDesign.PrintShop:
      // This case will need to be removed
}

Any code that stores the enum members directly, such as a fixture, will also need to be updated:

{
    ...
    format: {
        // With PrintShop removed, 20 will now refer to Obituary
        design: 20,
        ...
    }
}

Consideration will need to be given to what ArticleDesign will now be used for articles that were previously PrintShop. This is handled in the CAPI client for frontend/DCAR, and in AR itself for AR.

[changeset-bot]

🦋 Changeset detected

Latest commit: 8d25e21

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

Tip

Once this PR is ready to go, add the run_chromatic label to run the Chromatic tests.

This saves us a lot of money by not running the tests before we need them.

[github-actions]

Note

The following canaries were published to NPM:

• @guardian/[email protected]
• @guardian/[email protected]
• @guardian/[email protected]
• @guardian/[email protected]
• @guardian/[email protected]

🐥

[sndrs]

Are these types still needed in libs? Could they be moved to DCR directly?

[JamieB-gu]

Are these types still needed in libs? Could they be moved to DCR directly?

I'd like to do this. Two outstanding issues:

1. The source-development-kitchen/source-react-components-development-kitchen in CSNX depends on them.
2. AR depends on them.

I think we can work around 2), less sure about 1).

[github-actions]

Note

The following canaries were published to NPM:

• @guardian/[email protected]
• @guardian/[email protected]
• @guardian/[email protected]
• @guardian/[email protected]
• @guardian/[email protected]

🐥

[sndrs]

1. The source-development-kitchen/source-react-components-development-kitchen in CSNX depends on them.

given this is in inherently in a state of flux, could this just be updated when the types change enough if the lived in DCR? the same change would need to be accommodated with them living in libs anyway, is that right?

maybe if a component depends on a type which belongs in a product, the component also really belongs in that project? are the components that use these types used outside DCR do you know?

[JamieB-gu]

given this is in inherently in a state of flux, could this just be updated when the types change enough if the lived in DCR? the same change would need to be accommodated with them living in libs anyway, is that right?

Are you suggesting creating a duplicate of the type in the kitchen? In theory TypeScript's structural typing would consider them the same type if they have the same shape, but we might have to test to make sure? Alternatively we could say components that live in the kitchen just can't depend on this type, as you suggest in:

maybe if a component depends on a type which belongs in a product, the component also really belongs in that project? are the components that use these types used outside DCR do you know?

I think this is probably true! I can't find any meaningful references to ArticleFormat outside the guardian/dotcom-rendering or guardian/csnx repos. Hypothetically, the only other scenario I can think of is another library that's used in AR or DCAR, like e.g. support-dotcom-components, that might want access to a definition of this type? However, as mentioned, I can't find any concrete examples of this happening.

[sndrs]

I can't find any meaningful references to ArticleFormat outside the guardian/dotcom-rendering or guardian/csnx repos. Hypothetically, the only other scenario I can think of is another library that's used in AR or DCAR, like e.g. support-dotcom-components, that might want access to a definition of this type? However, as mentioned, I can't find any concrete examples of this happening.

how would you feel if i removed these things from libs/source-development-kitchen in that case?

it adds some complexity to these libraries to have this DCR-specific things in them, but at this point, i'm not sure what value it brings?

[JamieB-gu]

how would you feel if i removed these things from libs/source-development-kitchen in that case?

Is the proposal to remove all usage of ArticleFormat from these two libraries? So the kitchen components will no longer be able to have it as a prop?

[sndrs]

did you mean to merge this?

[sndrs]

Is the proposal to remove all usage of ArticleFormat from these two libraries? So the kitchen components will no longer be able to have it as a prop?

that's what i'm thinking, yeah.

EditorialButton sounds very much like it should live in an editorial application to me, and afaict is only consumed by DCR.

ToggleSwitch only switches styles based on the (optional) presence of Format, so it should probably be something like an isStyled optional boolean prop, rather than require a DCR-specific data type to toggle its styles.

QuoteIcon would accept a colour value as a theme or also move to DCR (if it will only ever need to have DCR-specific data types as variants - only DCR uses it currently).

If these components are needed outside a context where format makes sense, they should not really depend on it.

if the opposite, it seems weird to keep them somewhere where it has to imported to make them work

[JamieB-gu]

did you mean to merge this?

I did, yes. I know we're discussing moving ArticleFormat into DCAR, but I didn't necessarily want to block the print shop work behind that for an unknown length of time, given they're not technically coupled together. However, if you'd rather get that done first, and we can make the kitchen changes now, happy to discuss!

EditorialButton sounds very much like it should live in an editorial application to me, and afaict is only consumed by DCR.

ToggleSwitch only switches styles based on the (optional) presence of Format, so it should probably be something like an isStyled optional boolean prop, rather than require a DCR-specific data type to toggle its styles.

QuoteIcon would accept a colour value as a theme or also move to DCR (if it will only ever need to have DCR-specific data types as variants - only DCR uses it currently).

All sounds reasonable to me 👍