Add SpecialReportAlt card styles #25602

ioannakok · 2022-10-20T15:35:28Z

What does this change?

Consumes the new SpecialReportAltTheme coming from capi-client
Consumes the new SpecialReportAlt CardStyle coming from facia-client
Adds SpecialReportAlt card styles
Consumes new SpecialReportAltPalette (pending: https://github.com/guardian/interactive-atom-container-colours/pull/4)
When there is a campaign connected to a specific tag, if the campaign id is "SpecialReportAlt" facia-press decides that CardStyle will be SpecialReportAlt.

Previous PRs

Adds SpecialReportAltTheme and SpecialReportAlt hashed tag to content-api-scala-client: Add SpecialReportAltTheme to Format content-api-scala-client#371
Adds SpecialReportAltPalette: adds specialReport palette to fronts facia-scala-client#281
Adds SpecialReportAlt to CardStyle: Add SpecialReportAlt to CardStyle facia-scala-client#280
Bump capi and facia client int frontend: Bump content-api-scala-client to 19.0.5 and facia-scala-client to 4.0.3 #25640

Does this change need to be reproduced in dotcom-rendering ?

No
Yes (please indicate your plans for DCR Implementation)

The following PRs have already been merged:

Consumes the new SpecialReportAltTheme and adds SpecialReportAlt articles styles: Add articles styles for SpecialReportAlt dotcom-rendering#6313
Adds new SpecialReportAlt card styles: Add cards styles for SpecialReportAlt dotcom-rendering#6314
Consumes the new SpecialReportAltPalette and adds styles: Add SpecialReportAltPalette dotcom-rendering#6335

Screenshots

Standard type

Comment type

With media type

What is the value of this and can you measure success?

Checklist

Does this affect other platforms?

AMP
Apps
Other (please specify)

Does this affect GLabs Paid Content Pages? Should it have support for Paid Content?

No
Yes (please give details)

Does this change break ad-free?

No
It did, but tests caught it and I fixed it
It did, but there was no test coverage so I added that then fixed it

Does this change update the version of CAPI we're using?

No, all the existing database files are just fine
Yes, and I have re-run all the tests locally and checked in all the updated data/database/xyz files

Accessibility test checklist

Tested

Locally
On CODE (optional)

Tested in CODE and verified these changes don't break any existing card styles.

Further testing: At the moment we have no SpecialReportAlt articles published. Once this gets merged we are planning to do an e2e test "Tools Game" in CODE environment and

create articles of type:

Comment
Analysis
Explainer
Immersive
Standard

with different pillars to make sure all are being overridden correctly

Create a container using SpecialReportAltPalette and add SpecialReportAlt articles
Create onwards source using SpecialReportAlt articles

ioannakok · 2022-11-04T11:08:49Z

common/app/model/CardStylePicker.scala

    val tags = content.tags.map(_.id).toSeq
    extractCampaigns(tags) match {
      case Nil => CardStyle(content, TrailMetaData.empty)
+      case campaign:: _ => if(campaign.id.toString.toLowerCase() == "specialReportAlt") SpecialReportAlt else SpecialReport


Suggested change

case campaign:: _ => if(campaign.id.toString.toLowerCase() == "specialReportAlt") SpecialReportAlt else SpecialReport

case campaign:: _ => if(campaign.id.toString.toLowerCase() == "specialreportalt") SpecialReportAlt else SpecialReport

ioannakok · 2022-11-04T11:08:58Z

common/app/model/CardStylePicker.scala

@@ -21,6 +22,7 @@ object CardStylePicker {
    val tags = FaciaContentUtils.tags(content).map(_.id)
    extractCampaigns(tags) match {
      case Nil => FaciaContentUtils.cardStyle(content)
+      case campaign:: _ => if(campaign.id.toString.toLowerCase() == "specialReportAlt") SpecialReportAlt else SpecialReport


Suggested change

case campaign:: _ => if(campaign.id.toString.toLowerCase() == "specialReportAlt") SpecialReportAlt else SpecialReport

case campaign:: _ => if(campaign.id.toString.toLowerCase() == "specialreportalt") SpecialReportAlt else SpecialReport

ioannakok · 2022-11-04T12:54:32Z

common/app/model/meta.scala

@@ -286,6 +286,7 @@ object ContentFormat {
      case "CulturePillar"      => CulturePillar
      case "LifestylePillar"    => LifestylePillar
      case "SpecialReportTheme" => SpecialReportTheme
+//      case "SpecialReportAltTheme" => SpecialReportAltTheme


Suggested change

// case "SpecialReportAltTheme" => SpecialReportAltTheme

case "SpecialReportAltTheme" => SpecialReportAltTheme

Co-Authored-By: Ioanna Kokkini <ioannakok@users.noreply.github.com>

And remove unused parameter

… when tone is comment

ioannakok · 2022-11-09T15:12:47Z

common/app/model/CardStylePicker.scala

    val tags = content.tags.map(_.id).toSeq
    extractCampaigns(tags) match {
      case Nil => CardStyle(content, TrailMetaData.empty)
-      case _   => SpecialReport
+      case campaign :: _ =>
+        if (campaign.id.toString.toLowerCase() == "specialreportalt") SpecialReportAlt else SpecialReport


The campaign information is coming from extractCampaigns --> CampaignAgent --> targeting-client. There is already a campaign set up with this campaign id in the targeting-tool

ioannakok · 2022-11-09T15:12:57Z

common/app/model/CardStylePicker.scala

    }
  }

  def apply(content: FaciaContent): CardStyle = {
    val tags = FaciaContentUtils.tags(content).map(_.id)
    extractCampaigns(tags) match {
      case Nil => FaciaContentUtils.cardStyle(content)
-      case _   => SpecialReport
+      case campaign :: _ =>
+        if (campaign.id.toString.toLowerCase() == "specialreportalt") SpecialReportAlt else SpecialReport


Same as above

ioannakok · 2022-11-09T15:13:30Z

common/app/services/FaciaContentConvert.scala

@@ -13,7 +13,7 @@ object FaciaContentConvert {
  def contentToFaciaContent(content: Content): PressedContent = {
    val frontendContent = model.Content(content)
    val trailMetaData = TrailMetaData.empty
-    val cardStyle = CardStylePicker(content, trailMetaData)
+    val cardStyle = CardStylePicker(content)


This is just removing an unused parameter

ioannakok · 2022-11-09T15:14:26Z

common/app/views/support/GetClasses.scala

@@ -192,6 +194,7 @@ object GetClasses {
    BreakingPalette -> "fc-container--breaking-palette",
    EventPalette -> "fc-container--event-palette",
    EventAltPalette -> "fc-container--event-alt-palette",
+    SpecialReportAltPalette -> "fc-container--special-report-alt-palette",


There is a PR to add styles for this class.

ioannakok · 2022-11-09T15:15:17Z

static/src/stylesheets/module/facia-garnett/_pillars.scss

@@ -183,7 +197,7 @@ $pillars: (
        color: map-get($palette, kicker);
    }

-    &.fc-item--type-immersive.fc-item--has-boosted-title:not(.fc-item--pillar-special-report) .fc-item__standfirst {
+    &.fc-item--type-immersive.fc-item--has-boosted-title:not(.fc-item--pillar-special-report):not(.fc-item--pillar-special-report-alt) .fc-item__standfirst {


I tried

:not(.fc-item--pillar-special-report, .fc-item--pillar-special-report-alt)

but didn't work!

ioannakok · 2022-11-09T15:17:15Z

static/src/stylesheets/module/facia-garnett/_pillars.scss

+    background-color: $special-report-alt-faded !important;
+
+    .fc-item__container.u-faux-block-link--hover {
+        background-color: $special-report-alt-faded !important;


I am sorry I had to add !important to so many places but it wouldn't pick up the right colour for SpecialReportAlt. Given that fronts migration to DCR is in progress I don't think this is a huge problem but happy to discuss if anyone thinks otherwise!

HarryFischer

Looks good!

mxdvl

As you have mentioned, there are a few issues with the CSS styles, mainly the abundant use of !important, but this code base is soon to be legacy, so I believe it’s an acceptable compromise.

mxdvl · 2022-11-15T11:08:58Z

static/src/stylesheets/module/facia-garnett/_pillars.scss

+    background-color: $special-report-alt-faded !important;
+
+    .fc-item__container.u-faux-block-link--hover {
+        background-color: $special-report-alt-faded !important;


bryophyta · 2022-11-15T13:55:07Z

common/app/views/fragments/containers/facia_cards/standardContainer.scala.html

@@ -5,6 +5,8 @@
 @import views.support.RenderClasses
 @import conf.switches.Switches.AusRegionSelector

+@import views.html.fragments.containers.facia_cards.slice


are these imports here intentionally? (I couldn't spot any new code that makes use of them)

You're right! There is existing code that uses them and IntelliJ was complaining about the missing imports although the code was compiling successfully 🤷‍♀️

I thought I'd add them just because not having the imports didn't feel right although indeed it doesn't really make a difference!

ioannakok force-pushed the ioanna/cards-special-report-alt branch from 67f0a14 to efe1db1 Compare October 24, 2022 10:12

ioannakok mentioned this pull request Oct 28, 2022

Update the cards & article colour theme/palette for special report guardian/dotcom-rendering#6115

Closed

8 tasks

ioannakok force-pushed the ioanna/cards-special-report-alt branch from bc0da19 to ec7b2ec Compare October 31, 2022 10:24

ioannakok mentioned this pull request Oct 31, 2022

Add SpecialReportAlt to CardStyle guardian/facia-scala-client#280

Merged

7 tasks

ioannakok changed the title ~~WIP: Add special report alt card styles~~ Add SpecialReportAlt card styles Oct 31, 2022

ioannakok mentioned this pull request Nov 2, 2022

Add SpecialReportAltTheme to Format guardian/content-api-scala-client#371

Merged

ioannakok commented Nov 4, 2022

View reviewed changes

ioannakok mentioned this pull request Nov 8, 2022

Bump content-api-scala-client to 19.0.5 and facia-scala-client to 4.0.3 #25640

Merged

17 tasks

ioannakok and others added 10 commits November 8, 2022 11:10

WIP: Add special report alt card styles

c04b279

WIP: Add special report alt card styles

ca6f376

Add specialReportAlt CardStyle

16dc237

Co-Authored-By: Ioanna Kokkini <ioannakok@users.noreply.github.com>

Update CardStylePicker.scala

05c598d

Co-Authored-By: Ioanna Kokkini <ioannakok@users.noreply.github.com>

Update CardStylePicker.scala

3d413b4

Co-Authored-By: Ioanna Kokkini <ioannakok@users.noreply.github.com>

card styles update

f3513e6

tidy

18460d2

Add case for SpecialReportAlt in facia-press CardStylePicker

9c13516

Pattern match on specialReportAlt campaign id in CardStylePicker

e7439da

And remove unused parameter

Style SpecialReportAlt cards with media

3af4466

ioannakok force-pushed the ioanna/cards-special-report-alt branch from c35acc6 to 3af4466 Compare November 8, 2022 11:11

ioannakok added 4 commits November 8, 2022 11:15

Match to campaign id toLowerCase() in facia-press for SpecialReportAlt

beebe78

Send SpecialReportAltTheme to DCR

a15d29a

Run scalafmt

45a64f4

Add class for SpecialReportAltPalette

096df71

ioannakok changed the base branch from main to update-scala-facia-clients November 8, 2022 13:59

ioannakok added 3 commits November 8, 2022 15:09

Fix SpecialReportAlt sublinks and make sure styles are important even…

9717e65

… when tone is comment

Fix card gray lines for SpecialReportAlt when tone is comment

dffa3f3

Use rgba for opacity

a97af14

Base automatically changed from update-scala-facia-clients to main November 9, 2022 11:04

Fix comment count svg in SpecialReportAlt

b7deaa9

ioannakok force-pushed the ioanna/cards-special-report-alt branch from 3621d40 to b7deaa9 Compare November 9, 2022 13:02

ioannakok commented Nov 9, 2022

View reviewed changes

Fix byline and inline icons for SpecialReportAlt cards

4144a74

ioannakok marked this pull request as ready for review November 9, 2022 15:26

ioannakok requested a review from a team as a code owner November 9, 2022 15:26

ioannakok requested a review from HarryFischer November 9, 2022 15:29

HarryFischer approved these changes Nov 9, 2022

View reviewed changes

mxdvl approved these changes Nov 15, 2022

View reviewed changes

bryophyta reviewed Nov 15, 2022

View reviewed changes

ioannakok mentioned this pull request Nov 15, 2022

Add SpecialReportAltPalette #25680

Merged

2 tasks

ioannakok merged commit 68ef4d2 into main Nov 16, 2022

ioannakok deleted the ioanna/cards-special-report-alt branch November 16, 2022 11:43

ioannakok mentioned this pull request Nov 16, 2022

Revert "Add SpecialReportAlt card styles" #25687

Merged

prout-bot added the Seen-on-PROD label Nov 17, 2022

This was referenced Nov 17, 2022

Consume CardStyle SpecialReportAlt and SpecialReportAltTheme #25692

Merged

Add SpecialReportAlt card styles #25693

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add SpecialReportAlt card styles #25602

Add SpecialReportAlt card styles #25602

ioannakok commented Oct 20, 2022 •

edited

Loading

ioannakok Nov 4, 2022

ioannakok Nov 4, 2022

ioannakok Nov 4, 2022

ioannakok Nov 9, 2022 •

edited

Loading

ioannakok Nov 9, 2022

ioannakok Nov 9, 2022 •

edited

Loading

ioannakok Nov 9, 2022 •

edited

Loading

ioannakok Nov 9, 2022 •

edited

Loading

ioannakok Nov 9, 2022 •

edited

Loading

mxdvl Nov 15, 2022

HarryFischer left a comment

mxdvl left a comment

mxdvl Nov 15, 2022

bryophyta Nov 15, 2022

ioannakok Nov 16, 2022

	case campaign:: _ => if(campaign.id.toString.toLowerCase() == "specialReportAlt") SpecialReportAlt else SpecialReport
	case campaign:: _ => if(campaign.id.toString.toLowerCase() == "specialreportalt") SpecialReportAlt else SpecialReport

	// case "SpecialReportAltTheme" => SpecialReportAltTheme
	case "SpecialReportAltTheme" => SpecialReportAltTheme

Add SpecialReportAlt card styles #25602

Add SpecialReportAlt card styles #25602

Conversation

ioannakok commented Oct 20, 2022 • edited Loading

What does this change?

Previous PRs

Does this change need to be reproduced in dotcom-rendering ?

Screenshots

Standard type

Comment type

With media type

What is the value of this and can you measure success?

Checklist

Does this affect other platforms?

Does this affect GLabs Paid Content Pages? Should it have support for Paid Content?

Does this change break ad-free?

Does this change update the version of CAPI we're using?

Accessibility test checklist

Tested

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ioannakok Nov 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ioannakok Nov 9, 2022 • edited Loading

Choose a reason for hiding this comment

ioannakok Nov 9, 2022 • edited Loading

Choose a reason for hiding this comment

ioannakok Nov 9, 2022 • edited Loading

Choose a reason for hiding this comment

ioannakok Nov 9, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HarryFischer left a comment

Choose a reason for hiding this comment

mxdvl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ioannakok commented Oct 20, 2022 •

edited

Loading

ioannakok Nov 9, 2022 •

edited

Loading

ioannakok Nov 9, 2022 •

edited

Loading

ioannakok Nov 9, 2022 •

edited

Loading

ioannakok Nov 9, 2022 •

edited

Loading

ioannakok Nov 9, 2022 •

edited

Loading