jamesgorrie commented 1 year ago

We need to investigate where we are with palette tags. There was quite a lot of work done here https://github.com/guardian/dotcom-rendering/pull/4796.

Replicating the task list from there:

- [ ] https://github.com/guardian/dotcom-rendering/issues/4792
- [ ] https://github.com/guardian/dotcom-rendering/issues/4794
- [ ] https://github.com/guardian/dotcom-rendering/issues/4795

ioannakok commented 1 year ago

These are old tickets and most likely things have been fixed. We can test everything looks as expected in all the palettes though. Steps:

Go to: https://fronts.code.dev-gutools.co.uk/editorial/config in /break-this-front front Parisa Test container
You can see how it looks here: https://m.code.dev-theguardian.com/break-this-front
Add a Palette tag (e.g. SombreAltPalette), click "Save container". No need to test tags that do not have the Palette suffix.

Compare frontend (https://m.code.dev-theguardian.com/break-this-front?dcr=false) with DCR version (https://m.code.dev-theguardian.com/break-this-front?dcr=true)

ParisaTork commented 1 year ago

Control (no tags) looks good and we have relative parity between frontend and DCR (Frontend has 1 coloured headline and DCR has comments):

Frontend:

DCR:

Will test all tags individually in due course.

ParisaTork commented 1 year ago

SombreAltPalette is different from control and we don't have parity between frontend and DCR.

Frontend:

DCR:

Visual Differences:

Colouring of lines on the sides of the container (faint grey on frontend vs white on DCR)
Colouring of lines between cards (white on frontend vs grey on DCR)
Lines on top sides of cards (thicker on frontend vs thinner on DCR)
Dates (absent on frontend vs present on DCR)
Comments (absent on frontend vs present on DCR)
Grill lines on bottom of cards (full-width of bottom on frontend vs cut off by dates and comments on DCR)

Behavioural Differences:

Cards darken on-hover (darker on DCR)

alinaboghiu commented 1 year ago

BTW @ParisaTork if you run storybook locally there is a story for each palette at the bottom so you don't have to create an actual container every time - hopefully this helps (from dotcom-rendering root directory you run yarn storybook)

alinaboghiu commented 1 year ago

Just FYI another on dynamic/package there there is a wrong background colour that is also wrong in Frontend. Only happens when no palette is provided, this is being looked at separately: https://github.com/guardian/dotcom-rendering/issues/7951

ParisaTork commented 1 year ago

More Palette Analysis*

EventAltPalette

Frontend

DCR

Visual Differences:

Colouring of lines on the sides of the container (darker grey on frontend vs lighter grey on DCR)
Lines on top sides of cards (thicker on frontend vs thinner on DCR)
Dates (absent on frontend vs present on DCR)
Comments (absent on frontend vs present on DCR)
Grill lines on bottom of cards (full-width of bottom on frontend vs cut off by dates and comments on DCR)

Behavioural Differences:

Card tone changes on-hover (lighter on frontend vs darker on DCR)

InvestigationPalette

Frontend

DCR

Visual Differences:

Colouring of lines on the sides of the container (faint grey on frontend vs white on DCR)
Colouring of lines between cards (white on frontend vs grey on DCR)
Dates (absent on frontend vs present on DCR)
Comments (absent on frontend vs present on DCR)
Grill lines on bottom of cards (full-width of bottom on frontend vs cut off by dates and comments on DCR)

Behavioural Differences:

Cards darken on-hover (darker on DCR)

LongRunningAltPalette

Frontend

DCR

Visual Differences:

Colouring of lines on the sides of the container (darker grey on frontend vs lighter grey on DCR)
Colouring of lines between cards (light grey on frontend vs dark grey on DCR)
Dates (absent on frontend vs present on DCR)
Comments (absent on frontend vs present on DCR)
Grill lines on bottom of cards (none on frontend vs cut off by dates and comments on DCR)

Behavioural Differences:

Cards change tone on-hover (lighter on frontend vs darker on DCR)

SpecialReportAltPalette

Frontend

DCR

Visual Differences:

Colouring of lines on the sides of the container (dark grey on frontend vs light grey on DCR)
Colouring of lines between cards (light grey on frontend vs dark grey on DCR)
Dates (absent on frontend vs present on DCR)
Comments (absent on frontend vs present on DCR)
Grill lines on bottom of cards (full-width of bottom on frontend vs cut off by dates and comments on DCR)

Behavioural Differences:

Card tone changes on-hover (lighter on frontend vs darker on DCR)

EventPalette

Frontend

DCR

Visual Differences:

Colouring of lines on the sides of the container (dark grey on frontend vs light grey on DCR)
Lines on top sides of cards (thicker on frontend vs thinner on DCR)
Dates (absent on frontend vs present on DCR)
Comments (absent on frontend vs present on DCR)
Grill lines on bottom of cards (full-width of bottom on frontend vs cut off by dates and comments on DCR) (grill lines faint grey on frontend vs black on DCR)

Behavioural Differences:

Card tone changes on-hover (lighter on frontend vs darker on DCR)

SombrePalette

Frontend

DCR

Visual Differences:

Colouring of lines on the sides of the container (faint grey on frontend vs white on DCR)
Colouring of lines between cards (white on frontend vs grey on DCR)
Lines on top sides of cards (thicker on frontend vs thinner on DCR)
Dates (absent on frontend vs present on DCR)
Comments (absent on frontend vs present on DCR)
Grill lines on bottom of cards (full-width of bottom on frontend vs cut off by dates and comments on DCR)

Behavioural Differences:

Cards darken on-hover (darker on DCR)

* = Since time of writing, the date e.g. Friday 23rd June has been added on the left side of all containers in frontend and DCR

alinaboghiu commented 1 year ago

Thank you for these Parisa, very clear!

Comments -> confirmed as OK in this issue
Cards shade on-hover (darker on DCR) -> this is intentional for accessibility contrast reasons so OK

For the other points raised @HarryFischer if you have any free time please could you have a look through them and let us know if any of them are expected? These are:

border and vertical lines colouring
thickness of top line of cards
grill lines on bottom of cards

@ParisaTork until Harry comes back on these, we don't need to do anything else on this card.

ParisaTork commented 1 year ago

In tabular form:	Palette Name	Side of Container Line Colour (Frontend vs DCR)	Line Colour Between Cards (Frontend vs DCR)	Line Thickness Top Side of Cards (Frontend vs DCR)	Dates (Frontend vs DCR)	Comments (Frontend vs DCR)	Grill Lines on Bottom of Cards (Frontend vs DCR)
SombreAltPalette	Faint grey vs White 🐦	White vs Grey 🐱	Thicker vs Thinner 🦁	Absent vs Present 🐵	Absent vs Present ✅	Full-width vs Cut off 🐯	Cards darken (darker on DCR) ✅
EventAltPalette	Darker grey vs Lighter grey 🦒	Parity ✅	Thicker vs Thinner 🦁	Absent vs Present 🐵	Absent vs Present ✅	Full-width vs Cut off 🐯	Card tone changes (darker on DCR) ✅
InvestigationPalette	Faint grey vs White 🐦	White vs Grey 🐱	Parity ✅	Absent vs Present 🐵	Absent vs Present ✅	Full-width vs Cut off 🐯	Cards darken (darker on DCR) ✅
LongRunningAltPalette	Darker grey vs Lighter grey 🦒	Light grey vs Dark grey 🦊	Parity ✅	Absent vs Present 🐵	Absent vs Present ✅	None vs Cut off 🦓	Card tone changes (darker on DCR) ✅
SpecialReportAltPalette	Dark grey vs Lighter grey 🦒	Light grey vs Dark grey 🦊	Parity ✅	Absent vs Present 🐵	Absent vs Present ✅	Full-width vs Cut off 🐯	Card tone changes (darker on DCR) ✅
EventPalette	Dark grey vs Lighter grey 🦒	Parity ✅	Thicker vs Thinner 🦁	Absent vs Present 🐵	Absent vs Present ✅	Full-width (faint grey vs black) vs Cut off 🐯	Card tone changes (darker on DCR) ✅
SombrePalette	Faint grey vs White 🐦	White vs Grey 🐱	Thicker vs Thinner 🦁	Absent vs Present 🐵	Absent vs Present ✅	Full-width vs Cut off 🐯	Cards darken (darker on DCR) ✅

✅ = Fine / No Action Needed Animal emoji = Similar value

rhiannareechaye commented 1 year ago

Great summary, thanks @ParisaTork. I've taken your name off this for now and added Harry. I'm also chatting Harry about this so will comment here when i've got the answers

ParisaTork commented 1 year ago

Thanks Rhianna - Harry's going to send us the Figma design, but his calendar says that he's off today - he's going to request very minor colour changes to be made to the palettes - I worked on the analysis for this epic, so I'm going to add my name back and I've told him I'm happy to make the changes once he sends over the designs

rhiannareechaye commented 1 year ago

Oh perfect! Thanks @ParisaTork - apologies I know you said this in standup, I got confused today by looking at so many tickets during refinement, sounds great

HarryFischer commented 1 year ago

@ParisaTork @rhiannareechaye

border and vertical lines colouring — it's hard to tell the colours from screenshots — can you take the colours from here? https://www.figma.com/file/agVeiMZULSWlf4JxNgkng4/Fronts-Palettes?type=design&node-id=1%3A2&mode=design&t=Z1wgiv4zwdcnXkRQ-1
thickness of top line of cards — I think we should keep these all 1px - so DCR correct
grill lines on bottom of cards — it's correct that they now get chopped by the comment count and date

rhiannareechaye commented 1 year ago

Thanks Harry. FYI have moved https://github.com/guardian/dotcom-rendering/issues/4793 into our regular bugs milestone so that it can be triaged alongside our other bugs

VDuczekW commented 1 year ago

@ParisaTork please ticket up the outstanding actions and add to Rota board and then close this ticket

ParisaTork commented 1 year ago

@DanielCliftonGuardian & I just paired on this and one part of it is now done: https://github.com/guardian/dotcom-rendering/pull/8350 - I just need to look compare border & vertical styling now to see if we need to do anything on that

ParisaTork commented 1 year ago

Palettes - Outstanding Changes to be fixed for Border & Vertical Styling:

[ ] SombreAltPalette - Border Styling
[ ] InvestigationPalette - Border Styling
[ ] LongRunningAltPalette - Border Styling
[ ] SpecialReportAltPalette - Border Styling, Vertical Styling
[ ] SombrePalette - Border Styling

Palette Border & Vertical Styling Analysis Table:	Palette Name	Border Styling (Design)	Border Styling (DCR)	Border Styling (Parity?)	Vertical Styling (Design)	Vertical Styling (DCR)
SombreAltPalette	#FFFFFF 🐼	#707070 🐘	❌	#707070 🐘	#707070 🐘	✅
EventAltPalette	#dcdcdd 🦒	#dcdcdc 🐰	✅ (close enough)	#dcdcdc 🐰	#dcdcdd 🦒	✅ (close enough)
InvestigationPalette	#dcdcdc 🐰	#999999 🐧	❌	#999999 🐧	#999999 🐧	✅
LongRunningAltPalette	#dcdcdc 🐰	#cecece 🐨	❌	#cecece 🐨	#cecece 🐨	✅
SpecialReportAltPalette	#dcdcdc 🐰	#dad6d3😺	❌	#707070 🐘	#d8d6d2-#d8d6d3 🦁	❌
EventPalette	#dcdcdc 🐰	#dcdcdc 🐰	✅	#dcdcdc 🐰	#dcdcdc 🐰	✅
SombrePalette	#dcdcdc 🐰	#999999 🐧	❌	#999999 🐧	#999999 🐧	✅

✅ = Fine / No Action Needed Animal emoji = Similar value

N.B. For EventAltPalette, the values were close enough, so have labelled as ✅ .

VDuczekW commented 1 year ago

James & Parisa to pair

abeddow91 commented 1 year ago

@HarryFischer ^^ It looks like the designs might be slightly off here. We use the same colours for the container border lines and vertical lines between cards. Would you mind telling me which colour we should use for the following?

SombreAltPalette InvestigationPalette LongRunningAltPalette SpecialReportAltPalette SombrePalette

thanks!!

HarryFischer commented 1 year ago

Lets go:

SombreAltPalette — neutral.46 InvestigationPalette — neutral.60 LongRunningAltPalette — neutral.60 SpecialReportAltPalette — neutral.60 SombrePalette — neutral.60

guardian / dotcom-rendering

[EPIC] Palette tags #7311

More Palette Analysis*

EventAltPalette

Frontend

DCR

InvestigationPalette

Frontend

DCR

LongRunningAltPalette

Frontend

DCR

SpecialReportAltPalette

Frontend

DCR

EventPalette

Frontend

DCR

SombrePalette

Frontend

DCR