Add Contextual comms A/B/C test #850

emmabeynon · 2018-03-27T09:47:01Z

For https://trello.com/c/h0AKz5g2/55-overview-card-for-building-the-experiment-on-govuk

This adds an A/B/C test for the contextual comms experiment. We are testing two campaign formats (Native and Blue Box) against a control of no campaign displayed. We have four campaigns to promote and have selected a group of pages for each campaign on which the test will be conducted. After this test has concluded we intend to tear this code out.

To view the test variations in the browser, navigate to https://government-frontend-pr-850.herokuapp.com/armed-forces-pension-calculator and use the ModHeader Chrome extension to set a Request Header:

Name: GOVUK-ABTest-ContextualComms
Value: BlueBoxCampaign, NativeCampaign or NoCampaign.

Page with variation "NoCampaign"

Page with variation "NativeCampaign"

Page with variation "BlueBoxCampaign"

Component guide for this PR:
https://government-frontend-pr-850.herokuapp.com/component-guide

steventux

A couple of minor issues, mostly component style namespacing.

steventux · 2018-03-27T10:38:19Z

app/assets/stylesheets/components/_native-campaign.scss

+$light-grey: #bfc1c3;
+$dark-grey: #999;
+
+.native-campaign {


I think these styles should be prefixed with app-c- eg. app-c-native-campaign

steventux · 2018-03-27T10:39:42Z

app/assets/stylesheets/components/_native-campaign.scss

+  padding: 20px 0 0;
+}
+
+.native-campaign span {


We should use BEM naming conventions instead of applying styles to elements.
See this example: https://github.com/alphagov/government-frontend/blob/master/app/assets/stylesheets/components/_contents-list.scss#L29

steventux · 2018-03-27T10:42:40Z

app/controllers/concerns/contextual_comms_ab_testable.rb

+  end
+
+  def check_your_pay_pages
+    %w(


It would be better in terms of performance to freeze these as constants and (optionally) return them via the methods.

steventux · 2018-03-27T10:51:19Z

app/assets/stylesheets/components/_blue-box-campaign.scss

+$blue: #005ea5;
+$white: #ffffff;
+
+.blue-box-campaign {


These component styles should be prefixed with app-c- as they are app level components.

sihugh · 2018-03-27T10:58:39Z

app/controllers/concerns/contextual_comms_ab_testable.rb

+  end
+
+  def set_test_response_header
+    contextual_comms_test_variant.configure_response(response)


Should this have a predicate around it? We should probably only set this in the test context - so maybe just on the pages listed below? Fastly will cache separate pages for each variant when this is set, so we should limit it to what is relevant.

Like:

government-frontend/app/controllers/concerns/guide_nav_ab_testable.rb

Line 36 in 3d82ddf

guide_test_variant.configure_response(response) if is_tested_guide?

sihugh · 2018-03-27T11:06:06Z

app/views/layouts/application.html.erb

@@ -25,6 +25,7 @@
  <% end %>
  <meta name="govuk:content-id" content="<%= @content_item.content_id %>" />
  <%= guide_test_variant.analytics_meta_tag.html_safe if is_tested_guide? %>
+  <%= contextual_comms_test_variant.analytics_meta_tag.html_safe %>


This is the other thing where we should perhaps only include if it's relevant. This is the thing that sends the analytics

vanitabarrett

A few frontend changes - I hope this helps (and I think it may help with the IE browser issues!). Let me know if you need me to clarify anything, happy to go through in person.

(My comments probably also apply to the native-campaign component, I only reviewed the blue-box one)

vanitabarrett · 2018-03-27T15:50:41Z

app/assets/stylesheets/components/_blue-box-campaign.scss

@@ -0,0 +1,45 @@
+$light-grey: #bfc1c3;


We should be able to use colours from GOVUK Elements: http://govuk-elements.herokuapp.com/colour/

I think:

$govuk-blue

$grey-[1/2/3/4]

$white

vanitabarrett · 2018-03-27T15:54:11Z

app/assets/stylesheets/components/_blue-box-campaign.scss

+$white: #ffffff;
+
+.app-c-native-blue-box-campaign__border {
+  display: block;


You shouldn't need this - div's render as blocks automatically.

vanitabarrett · 2018-03-27T15:54:23Z

app/assets/stylesheets/components/_blue-box-campaign.scss

+.app-c-native-blue-box-campaign__border {
+  display: block;
+  border-top: 1px solid $light-grey;
+  padding: 20px 0 0;


Can be simplified to padding-top: 20px

vanitabarrett · 2018-03-27T15:54:45Z

app/assets/stylesheets/components/_blue-box-campaign.scss

+  display: block;
+  border-top: 1px solid $light-grey;
+  padding: 20px 0 0;
+  text-align: center;


I don't think this is needed?

vanitabarrett · 2018-03-27T15:59:09Z

app/assets/stylesheets/components/_blue-box-campaign.scss

+}
+
+.app-c-native-blue-box-campaign__link {
+  display: flex;


This may be causing your problems in IE - it doesn't have very good flexbox support. I don't think this needs flexbox as it isn't too complex.

You should just be able to use position:relative; display: block; and delete the following lines:

display: flex; flex-direction: column; justify-content: space-between;

You'll then have to add the following lines to .app-c-native-blue-box-campaign__description to make it appear at the bottom of the div:

position: absolute; bottom: 10px;

Happy to chat through this in person!

vanitabarrett · 2018-03-27T16:01:21Z

app/assets/stylesheets/components/_blue-box-campaign.scss

+  display: block;
+  color: $white;
+
+  &:hover {


Just an FYI - the focus state when you click on the box doesn't meet accessible color contrast guidelines. You may want to consider changing the text to black on focus.

vanitabarrett · 2018-03-27T16:02:10Z

app/assets/stylesheets/components/_blue-box-campaign.scss

+  text-align: left;
+  box-sizing: border-box;
+  padding: 15px;
+  font-size: 27px;


I would suggest trying to use GOV.UK Elements font mixins, e.g: @include bold-24 in this case. I think this includes the line-height.

vanitabarrett · 2018-03-27T16:02:28Z

app/assets/stylesheets/components/_blue-box-campaign.scss

+  display: block;
+  font-weight: normal;
+  padding-top: 10px;
+  font-size: 19px;


As above: I would suggest trying to use GOV.UK Elements font mixins, e.g: @include core-19 in this case. I think this includes the line-height.

vanitabarrett · 2018-03-27T16:03:34Z

app/views/components/_blue-box-campaign.html.erb

@@ -0,0 +1,6 @@
+<div class="app-c-native-blue-box-campaign__border">


I'd suggest renaming this - although it does include border styling, this could be changed in future and the class could become misleading. It's more of a app-c-native-blue-box-campaign__container or app-c-native-blue-box-campaign__wrapper

vanitabarrett · 2018-03-27T16:06:11Z

app/views/components/_blue-box-campaign.html.erb

@@ -0,0 +1,6 @@
+<div class="app-c-native-blue-box-campaign__border">
+  <a class="app-c-native-blue-box-campaign__link" href="<%= href %>">
+    <span class="app-c-native-blue-box-campaign__title"><strong><%= title %></strong></span>


I'm not sure this is the right place to use <strong> tags, a heading element may be more appropriate.

vanitabarrett · 2018-03-27T16:19:13Z

Note: we normally require components to have documentation in the relevant app component guide too. I'm not sure if you want to do this if the components are only temporary, but there's some good documentation here on our components and conventions https://github.com/alphagov/govuk_publishing_components/blob/master/docs/component_conventions.md

emmabeynon · 2018-03-28T10:02:33Z

@vanitabarrett @steventux @sihugh Thanks for your reviews, your comments should be addressed now 👍

vanitabarrett

A couple of small CSS refactors, otherwise looks good to me! 👍

vanitabarrett · 2018-03-28T10:23:20Z

app/assets/stylesheets/components/_blue-box-campaign.scss

+  text-align: left;
+  box-sizing: border-box;
+  padding: 15px;
+  font-weight: bold;


This isn't needed if you're including bold-24.

vanitabarrett · 2018-03-28T10:25:00Z

app/assets/stylesheets/components/_native-campaign.scss

+  display: block;
+  text-decoration: none;
+  @include core-19;
+  font-weight: bold;


If you want it to be bold and 19px, you can include bold-19 instead of core-19. This means you won't need font-weight: bold

This is a temporary component to be used in the contextual communications A/B/C test.

This is a temporary component to be used in the contextual comms A/B/C test.

We add logic to display either a blue box campaign, a native campaign or no campaign at all depending on which test variant is set. For now, we hardcode the values being passed into the campaign components, but this will be changed in a future commit.

We update the controller logic to pass the correct title, description and link data to the campaign components when a page should display the "Get In Go Far" campaign.

We add data for the "Eating" campaign and a whitelist of pages on which the campaign is eligible to be displayed.

This ensures that pages that haven't been whitelisted to display a contextual comms campaign don't attempt to display one. We also ensure that whitelisted pages that have been assigned the 'NoCampaign' variant don't show a campaign.

This adds data to display the "Check Your Pay" campaign and whitelists the pages on which this campaign is eligible to appear.

This adds data to display the "Your Pension" campaign and whitelists the pages on which this campaign is eligible to appear.

These changes have been addressed.

tijmenb temporarily deployed to government-frontend-pr-850 March 27, 2018 09:47 Inactive

emmabeynon force-pushed the contextual-comms-abc-test branch from 1f28cb7 to 4d2afdf Compare March 27, 2018 10:02

boffbowsh temporarily deployed to government-frontend-pr-850 March 27, 2018 10:02 Inactive

emmabeynon force-pushed the contextual-comms-abc-test branch from 4d2afdf to d6937e3 Compare March 27, 2018 10:14

boffbowsh temporarily deployed to government-frontend-pr-850 March 27, 2018 10:16 Inactive

steventux previously requested changes Mar 27, 2018

View reviewed changes

sihugh reviewed Mar 27, 2018

View reviewed changes

emmabeynon force-pushed the contextual-comms-abc-test branch from d6937e3 to c5c609e Compare March 27, 2018 15:19

boffbowsh temporarily deployed to government-frontend-pr-850 March 27, 2018 15:19 Inactive

vanitabarrett suggested changes Mar 27, 2018

View reviewed changes

emmabeynon force-pushed the contextual-comms-abc-test branch from c5c609e to 875ce58 Compare March 28, 2018 10:01

boffbowsh temporarily deployed to government-frontend-pr-850 March 28, 2018 10:02 Inactive

vanitabarrett approved these changes Mar 28, 2018

View reviewed changes

emmabeynon force-pushed the contextual-comms-abc-test branch from 875ce58 to bc614b3 Compare March 28, 2018 10:35

boffbowsh temporarily deployed to government-frontend-pr-850 March 28, 2018 10:35 Inactive

emmabeynon force-pushed the contextual-comms-abc-test branch from bc614b3 to fe66024 Compare March 28, 2018 14:39

boffbowsh temporarily deployed to government-frontend-pr-850 March 28, 2018 14:39 Inactive

emmabeynon added 11 commits March 28, 2018 15:30

Add native campaign component

cc5a457

This is a temporary component to be used in the contextual communications A/B/C test.

Add blue box campaign component

94bf5db

This is a temporary component to be used in the contextual comms A/B/C test.

Add Contextual Comms AB Testable controller

b531152

Add basic AB test setup to Contextual Comms AB Testable

571b29e

Display contextual comms campaigns

334f7d7

We add logic to display either a blue box campaign, a native campaign or no campaign at all depending on which test variant is set. For now, we hardcode the values being passed into the campaign components, but this will be changed in a future commit.

Display Get In Go Far campaign

31b8eb2

We update the controller logic to pass the correct title, description and link data to the campaign components when a page should display the "Get In Go Far" campaign.

Display Eating campaign

caccc4b

We add data for the "Eating" campaign and a whitelist of pages on which the campaign is eligible to be displayed.

Handle pages that shouldn't display campaigns

efa5a8b

This ensures that pages that haven't been whitelisted to display a contextual comms campaign don't attempt to display one. We also ensure that whitelisted pages that have been assigned the 'NoCampaign' variant don't show a campaign.

Display Check Your Pay campaign

30f8436

This adds data to display the "Check Your Pay" campaign and whitelists the pages on which this campaign is eligible to appear.

Display Your Pension campaign

e3cc4b9

This adds data to display the "Your Pension" campaign and whitelists the pages on which this campaign is eligible to appear.

Add GA tracking to external campaign links

570621a

emmabeynon force-pushed the contextual-comms-abc-test branch from fe66024 to 570621a Compare March 28, 2018 15:32

boffbowsh deployed to government-frontend-pr-850 March 28, 2018 15:32 View deployment

emmabeynon merged commit 0831526 into master Mar 29, 2018

emmabeynon deleted the contextual-comms-abc-test branch March 29, 2018 09:01

tijmenb mentioned this pull request Apr 16, 2018

Temporarily disable "Get in go far" campaign #877

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Contextual comms A/B/C test #850

Add Contextual comms A/B/C test #850

emmabeynon commented Mar 27, 2018 •

edited

Loading

steventux left a comment

steventux Mar 27, 2018

steventux Mar 27, 2018

steventux Mar 27, 2018

steventux Mar 27, 2018

sihugh Mar 27, 2018

sihugh Mar 27, 2018

sihugh Mar 27, 2018

vanitabarrett left a comment •

edited

Loading

vanitabarrett Mar 27, 2018

vanitabarrett Mar 27, 2018

vanitabarrett Mar 27, 2018

vanitabarrett Mar 27, 2018

vanitabarrett Mar 27, 2018

vanitabarrett Mar 27, 2018

vanitabarrett Mar 27, 2018 •

edited

Loading

vanitabarrett Mar 27, 2018 •

edited

Loading

vanitabarrett Mar 27, 2018 •

edited

Loading

vanitabarrett Mar 27, 2018

vanitabarrett commented Mar 27, 2018

emmabeynon commented Mar 28, 2018

vanitabarrett left a comment

vanitabarrett Mar 28, 2018

vanitabarrett Mar 28, 2018

		@@ -0,0 +1,6 @@
		<div class="app-c-native-blue-box-campaign__border">

Add Contextual comms A/B/C test #850

Add Contextual comms A/B/C test #850

Conversation

emmabeynon commented Mar 27, 2018 • edited Loading

steventux 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

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

vanitabarrett left a comment • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vanitabarrett Mar 27, 2018 • edited Loading

Choose a reason for hiding this comment

vanitabarrett Mar 27, 2018 • edited Loading

Choose a reason for hiding this comment

vanitabarrett Mar 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vanitabarrett commented Mar 27, 2018

emmabeynon commented Mar 28, 2018

vanitabarrett left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emmabeynon commented Mar 27, 2018 •

edited

Loading

vanitabarrett left a comment •

edited

Loading

vanitabarrett Mar 27, 2018 •

edited

Loading

vanitabarrett Mar 27, 2018 •

edited

Loading

vanitabarrett Mar 27, 2018 •

edited

Loading