guardian / dotcom-rendering

The Guardian web rendering service (aka DCR/DCAR)
https://www.theguardian.com
Apache License 2.0
255 stars 30 forks source link

Remove special headline colours for Feature cards #7289

Closed bryophyta closed 1 year ago

bryophyta commented 1 year ago

Feature cards on Frontend use a dark version of their pillar colour for headlines.

It looks like DCR is currently implementing this logic, but imperfectly. The design is being changed so that feature cards should use the standard colours for their headlines (i.e. black, or white if on a dark background).

So we should check whether there is currently some logic trying to implement the special case for feature cards in DCR and remove it.

image

Update: It looks like we're setting this logic here at the moment. Hopefully it'll be as simple as removing this block, but a) we should check what we need to do with recipes and reviews, and b) we should check that this isn't going to introduce regressions in other components that might be using textHeadline (though hopefully Chromatic will catch that).

https://github.com/guardian/dotcom-rendering/blob/4dd60c3e74de345b0d1695aff330f6e573e47026/dotcom-rendering/src/web/lib/decidePalette.ts#L84-L88

AshCorr commented 1 year ago

Believe this is actually configured in textCardHeadline, not textHeadline

https://github.com/guardian/dotcom-rendering/blob/4ad20322d3937e7b105aed74b25d32ee979a58c7/dotcom-rendering/src/web/lib/decidePalette.ts#L637-L666

We don't currently have a special case for Feature cards in DCR.

bryophyta commented 1 year ago

Believe this is actually configured in textCardHeadline, not textHeadline

Ah yes, good call!

Going back over the notes from the design review I don't think I'm properly parsing the issue here. It looks like the request was either to replicate the Frontend behaviour for Features, or remove coloured headlines from all cards (including Interviews etc.?) to match behaviour on apps. But I'm not sure whether we got confirmation one way or another in the end -- are you able to provide an update here at all @HarryFischer?

HarryFischer commented 1 year ago

Alex Breuer was going to ask about this - that was the last I heard @bryophyta

AshCorr commented 1 year ago

I'll marked this as blocked for now, I'll reach out to Alex and see if they have any updates.

ajbreuer commented 1 year ago

@AshCorr Looking at this now, the change we are looking for applies to cards across all pillars. Lifestyle, Culture, Sport and news.

You can see in the images below, the coloured headlines on lifestyle, news and culture. And blow on sport features

Screenshot 2023-04-12 at 11 16 23 Screenshot 2023-04-12 at 11 16 47

All these headlines should become black so just the Quotemarks, kicker, and byline names pick up the pillar colour