From 626bb2af5088b220ed8c80e65cf6dcc07f29b161 Mon Sep 17 00:00:00 2001 From: Richard Towers Date: Fri, 29 Nov 2024 14:15:23 +0000 Subject: [PATCH 1/7] Rename hero--mid-page class to match theme name This also adds an app-b-hero--default class to default hero images. --- app/assets/stylesheets/views/_landing_page/hero.scss | 2 +- app/views/landing_page/blocks/_hero.html.erb | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/views/_landing_page/hero.scss b/app/assets/stylesheets/views/_landing_page/hero.scss index 9da9f82d22..deae1500c9 100644 --- a/app/assets/stylesheets/views/_landing_page/hero.scss +++ b/app/assets/stylesheets/views/_landing_page/hero.scss @@ -51,7 +51,7 @@ $large-desktop-height: 400px; } } -.app-b-hero--mid-page { +.app-b-hero--middle_left { overflow: hidden; @include govuk-media-query($until: desktop) { diff --git a/app/views/landing_page/blocks/_hero.html.erb b/app/views/landing_page/blocks/_hero.html.erb index 80eb08bd25..117afcee90 100644 --- a/app/views/landing_page/blocks/_hero.html.erb +++ b/app/views/landing_page/blocks/_hero.html.erb @@ -2,8 +2,7 @@ <% add_view_stylesheet("landing_page/hero") - hero_classes = %w[app-b-hero] - hero_classes << "app-b-hero--mid-page" if block.data["theme"] == "middle_left" + hero_classes = %W[app-b-hero app-b-hero--#{block.data.fetch("theme", "default")}] grid_class = "govuk-grid-column-two-thirds-from-desktop" grid_class = "govuk-grid-column-one-third-from-desktop" if block.data["theme"] == "middle_left" From 1e53f2ed5c22d6cdb0eb7f4cb9073cab0f7adcf9 Mon Sep 17 00:00:00 2001 From: Richard Towers Date: Fri, 29 Nov 2024 14:19:52 +0000 Subject: [PATCH 2/7] Pass font_size instead of position to headings This way headings don't need to know about their parents, and whether they're a top or not. Which lets face it, nobody wants to know. Instead, the hero block knows about headings so it can pass down some config to them. We could do this with CSS, as we're doing for govspeak, but the component isolation is better this way. --- app/views/landing_page/blocks/_featured.html.erb | 2 +- app/views/landing_page/blocks/_heading.html.erb | 2 +- app/views/landing_page/blocks/_hero.html.erb | 11 ++++++++--- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/app/views/landing_page/blocks/_featured.html.erb b/app/views/landing_page/blocks/_featured.html.erb index e9557b0f13..1a0bacd6db 100644 --- a/app/views/landing_page/blocks/_featured.html.erb +++ b/app/views/landing_page/blocks/_featured.html.erb @@ -9,7 +9,7 @@ <% block.featured_content.each do |subblock| %> <% options = {} - options[:position] = "top" if subblock.type == "heading" + options[:font_size] = "l" if subblock.type == "heading" %> <%= render_block(subblock, options:) %> <% end %> diff --git a/app/views/landing_page/blocks/_heading.html.erb b/app/views/landing_page/blocks/_heading.html.erb index a4ce2acbfd..05f40eadf1 100644 --- a/app/views/landing_page/blocks/_heading.html.erb +++ b/app/views/landing_page/blocks/_heading.html.erb @@ -1,6 +1,6 @@ <% heading_level = block.data["heading_level"] || 2 - font_size = options[:position] == "top" ? "l" : "m" + font_size = options[:font_size] text = block.data["content"] path = block.data["path"] || nil inverse = options[:inverse] || false diff --git a/app/views/landing_page/blocks/_hero.html.erb b/app/views/landing_page/blocks/_hero.html.erb index 117afcee90..a2adcb8b22 100644 --- a/app/views/landing_page/blocks/_hero.html.erb +++ b/app/views/landing_page/blocks/_hero.html.erb @@ -10,6 +10,10 @@ hero_textbox_classes = %w[app-b-hero__textbox] hero_textbox_classes << "border-top--#{style(block.data["theme_colour"])}" if block.data["theme_colour"] + hero_heading_options = { + font_size: ("l" if block.data["theme"].nil?), + }.compact + image_alt_text = block.image.alt || "" %> @@ -26,9 +30,10 @@ <%= content_tag("div", class: grid_class) do %> <%= content_tag("div", class: hero_textbox_classes) do %> <% block.hero_content.each_with_index do |subblock, index| %> - <% options = { - position: block.data["position"], - margin_bottom: (0 if block.hero_content.length - 1 == index) } %> + <% + options = { margin_bottom: (0 if block.hero_content.length - 1 == index) } + options.merge!(hero_heading_options) if subblock.type == "heading" + %> <%= render_block(subblock, options:) %> <% end %> <% end %> From 9275e9254fb4a5b2e8b6ced4a575c7bfa33cce31 Mon Sep 17 00:00:00 2001 From: Richard Towers Date: Fri, 29 Nov 2024 11:50:08 +0000 Subject: [PATCH 3/7] Remove defunct position: top config --- lib/data/landing_page_content_items/be_kinder.yaml | 1 - lib/data/landing_page_content_items/be_thankful.yaml | 1 - lib/data/landing_page_content_items/donate_to_charity.yaml | 1 - lib/data/landing_page_content_items/exercise_more.yaml | 1 - lib/data/landing_page_content_items/goals.yaml | 4 ++-- lib/data/landing_page_content_items/homepage.yaml | 1 - lib/data/landing_page_content_items/learn_something_new.yaml | 1 - lib/data/landing_page_content_items/tasks.yaml | 1 - 8 files changed, 2 insertions(+), 9 deletions(-) diff --git a/lib/data/landing_page_content_items/be_kinder.yaml b/lib/data/landing_page_content_items/be_kinder.yaml index b395738695..0d88ec0c09 100644 --- a/lib/data/landing_page_content_items/be_kinder.yaml +++ b/lib/data/landing_page_content_items/be_kinder.yaml @@ -59,7 +59,6 @@ blocks: - type: main_navigation navigation_group_id: Top Menu - type: hero - position: top image: alt: "Placeholder alt text" sources: diff --git a/lib/data/landing_page_content_items/be_thankful.yaml b/lib/data/landing_page_content_items/be_thankful.yaml index b9e4013cb1..d2ab6dde6d 100644 --- a/lib/data/landing_page_content_items/be_thankful.yaml +++ b/lib/data/landing_page_content_items/be_thankful.yaml @@ -59,7 +59,6 @@ blocks: - type: main_navigation navigation_group_id: Top Menu - type: hero - position: top image: alt: "Placeholder alt text" sources: diff --git a/lib/data/landing_page_content_items/donate_to_charity.yaml b/lib/data/landing_page_content_items/donate_to_charity.yaml index 1ca038b669..35e1d10bc4 100644 --- a/lib/data/landing_page_content_items/donate_to_charity.yaml +++ b/lib/data/landing_page_content_items/donate_to_charity.yaml @@ -59,7 +59,6 @@ blocks: - type: main_navigation navigation_group_id: Top Menu - type: hero - position: top image: alt: "Placeholder alt text" sources: diff --git a/lib/data/landing_page_content_items/exercise_more.yaml b/lib/data/landing_page_content_items/exercise_more.yaml index dacca6ca39..e0f618364a 100644 --- a/lib/data/landing_page_content_items/exercise_more.yaml +++ b/lib/data/landing_page_content_items/exercise_more.yaml @@ -59,7 +59,6 @@ blocks: - type: main_navigation navigation_group_id: Top Menu - type: hero - position: top image: alt: "Placeholder alt text" sources: diff --git a/lib/data/landing_page_content_items/goals.yaml b/lib/data/landing_page_content_items/goals.yaml index 20ea4f7142..7ee522d63f 100644 --- a/lib/data/landing_page_content_items/goals.yaml +++ b/lib/data/landing_page_content_items/goals.yaml @@ -45,7 +45,6 @@ blocks: - type: main_navigation navigation_group_id: Top Menu - type: hero - position: top image: sources: desktop: "landing_page/placeholder/desktop.png" @@ -60,7 +59,8 @@ blocks: content: Government Goals - type: govspeak content: | -

Culpa atque nostrum numquam eveniet. Cum exercitationem perferendis accusamus minima possimus dolor enim eius. Et est impedit vel voluptate sunt.

+

Culpa atque nostrum numquam eveniet. Cum exercitationem perferendis accusamus minima possimus dolor enim eius. Et est impedit vel voluptate sunt.

+

hello world

- type: two_column_layout theme: two_thirds_one_third blocks: diff --git a/lib/data/landing_page_content_items/homepage.yaml b/lib/data/landing_page_content_items/homepage.yaml index 0bef0bbba2..f0c69b0654 100644 --- a/lib/data/landing_page_content_items/homepage.yaml +++ b/lib/data/landing_page_content_items/homepage.yaml @@ -45,7 +45,6 @@ blocks: - type: main_navigation navigation_group_id: Top Menu - type: hero - position: top image: sources: desktop: "landing_page/placeholder/missions_hero_desktop.jpg" diff --git a/lib/data/landing_page_content_items/learn_something_new.yaml b/lib/data/landing_page_content_items/learn_something_new.yaml index b237ef572a..ff08a48065 100644 --- a/lib/data/landing_page_content_items/learn_something_new.yaml +++ b/lib/data/landing_page_content_items/learn_something_new.yaml @@ -59,7 +59,6 @@ blocks: - type: main_navigation navigation_group_id: Top Menu - type: hero - position: top image: alt: "Placeholder alt text" sources: diff --git a/lib/data/landing_page_content_items/tasks.yaml b/lib/data/landing_page_content_items/tasks.yaml index 3ecfa28790..dbde3bd442 100644 --- a/lib/data/landing_page_content_items/tasks.yaml +++ b/lib/data/landing_page_content_items/tasks.yaml @@ -57,7 +57,6 @@ blocks: - type: main_navigation navigation_group_id: Top Menu - type: hero - position: top image: sources: desktop: "landing_page/placeholder/desktop.png" From fb1da5256bad033262f21f06ca7479eb86592cf1 Mon Sep 17 00:00:00 2001 From: Richard Towers Date: Fri, 29 Nov 2024 11:56:31 +0000 Subject: [PATCH 4/7] Add theme attribute to hero models --- app/models/landing_page/block/hero.rb | 3 ++- app/views/landing_page/blocks/_hero.html.erb | 6 +++--- spec/models/landing_page/block/hero_spec.rb | 11 +++++++++++ 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/app/models/landing_page/block/hero.rb b/app/models/landing_page/block/hero.rb index b5776129bd..ce7f63bc0b 100644 --- a/app/models/landing_page/block/hero.rb +++ b/app/models/landing_page/block/hero.rb @@ -3,7 +3,7 @@ module LandingPage::Block HeroImage = Data.define(:alt, :sources) class Hero < Base - attr_reader :image, :hero_content + attr_reader :image, :hero_content, :theme def initialize(block_hash, landing_page) super @@ -12,6 +12,7 @@ def initialize(block_hash, landing_page) sources = HeroImageSources.new(**sources) @image = HeroImage.new(alt:, sources:) @hero_content = LandingPage::BlockFactory.build_all(data.dig("hero_content", "blocks"), landing_page) + @theme = data.fetch("theme", "default") end def full_width? diff --git a/app/views/landing_page/blocks/_hero.html.erb b/app/views/landing_page/blocks/_hero.html.erb index a2adcb8b22..0e8fa345ba 100644 --- a/app/views/landing_page/blocks/_hero.html.erb +++ b/app/views/landing_page/blocks/_hero.html.erb @@ -2,16 +2,16 @@ <% add_view_stylesheet("landing_page/hero") - hero_classes = %W[app-b-hero app-b-hero--#{block.data.fetch("theme", "default")}] + hero_classes = %W[app-b-hero app-b-hero--#{block.theme}] grid_class = "govuk-grid-column-two-thirds-from-desktop" - grid_class = "govuk-grid-column-one-third-from-desktop" if block.data["theme"] == "middle_left" + grid_class = "govuk-grid-column-one-third-from-desktop" if block.theme == "middle_left" hero_textbox_classes = %w[app-b-hero__textbox] hero_textbox_classes << "border-top--#{style(block.data["theme_colour"])}" if block.data["theme_colour"] hero_heading_options = { - font_size: ("l" if block.data["theme"].nil?), + font_size: ("l" if block.theme == "default"), }.compact image_alt_text = block.image.alt || "" diff --git a/spec/models/landing_page/block/hero_spec.rb b/spec/models/landing_page/block/hero_spec.rb index 4c8c228c1d..263a88f9de 100644 --- a/spec/models/landing_page/block/hero_spec.rb +++ b/spec/models/landing_page/block/hero_spec.rb @@ -41,6 +41,17 @@ end end + describe "#theme" do + it "defaults to default" do + expect(subject.theme).to eq("default") + end + + it "returns the theme from config" do + blocks_hash["theme"] = "middle_left" + expect(subject.theme).to eq("middle_left") + end + end + describe "#full_width?" do it "is true" do expect(subject.full_width?).to eq(true) From f1d87a85977b6672df4dcbad0106da38e435af1a Mon Sep 17 00:00:00 2001 From: Richard Towers Date: Fri, 29 Nov 2024 12:24:11 +0000 Subject: [PATCH 5/7] Add theme_colour attribute to hero models --- app/models/landing_page/block/hero.rb | 3 ++- app/views/landing_page/blocks/_hero.html.erb | 2 +- spec/models/landing_page/block/hero_spec.rb | 11 +++++++++++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/app/models/landing_page/block/hero.rb b/app/models/landing_page/block/hero.rb index ce7f63bc0b..e0d9f0ccbc 100644 --- a/app/models/landing_page/block/hero.rb +++ b/app/models/landing_page/block/hero.rb @@ -3,7 +3,7 @@ module LandingPage::Block HeroImage = Data.define(:alt, :sources) class Hero < Base - attr_reader :image, :hero_content, :theme + attr_reader :image, :hero_content, :theme, :theme_colour def initialize(block_hash, landing_page) super @@ -13,6 +13,7 @@ def initialize(block_hash, landing_page) @image = HeroImage.new(alt:, sources:) @hero_content = LandingPage::BlockFactory.build_all(data.dig("hero_content", "blocks"), landing_page) @theme = data.fetch("theme", "default") + @theme_colour = data["theme_colour"] end def full_width? diff --git a/app/views/landing_page/blocks/_hero.html.erb b/app/views/landing_page/blocks/_hero.html.erb index 0e8fa345ba..75f8f0c709 100644 --- a/app/views/landing_page/blocks/_hero.html.erb +++ b/app/views/landing_page/blocks/_hero.html.erb @@ -8,7 +8,7 @@ grid_class = "govuk-grid-column-one-third-from-desktop" if block.theme == "middle_left" hero_textbox_classes = %w[app-b-hero__textbox] - hero_textbox_classes << "border-top--#{style(block.data["theme_colour"])}" if block.data["theme_colour"] + hero_textbox_classes << "border-top--#{style(block.theme_colour)}" if block.theme_colour hero_heading_options = { font_size: ("l" if block.theme == "default"), diff --git a/spec/models/landing_page/block/hero_spec.rb b/spec/models/landing_page/block/hero_spec.rb index 263a88f9de..9e0cab5b98 100644 --- a/spec/models/landing_page/block/hero_spec.rb +++ b/spec/models/landing_page/block/hero_spec.rb @@ -52,6 +52,17 @@ end end + describe "#theme_colour" do + it "defaults to nil" do + expect(subject.theme_colour).to eq(nil) + end + + it "returns the theme_colour from config" do + blocks_hash["theme_colour"] = 2 + expect(subject.theme_colour).to eq(2) + end + end + describe "#full_width?" do it "is true" do expect(subject.full_width?).to eq(true) From b25bcdb24cd43f9116302bd11146835b08454a1e Mon Sep 17 00:00:00 2001 From: Richard Towers Date: Tue, 3 Dec 2024 10:02:49 +0000 Subject: [PATCH 6/7] Clarify subblock options --- app/views/landing_page/blocks/_hero.html.erb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/app/views/landing_page/blocks/_hero.html.erb b/app/views/landing_page/blocks/_hero.html.erb index 75f8f0c709..a62334dd3b 100644 --- a/app/views/landing_page/blocks/_hero.html.erb +++ b/app/views/landing_page/blocks/_hero.html.erb @@ -10,10 +10,6 @@ hero_textbox_classes = %w[app-b-hero__textbox] hero_textbox_classes << "border-top--#{style(block.theme_colour)}" if block.theme_colour - hero_heading_options = { - font_size: ("l" if block.theme == "default"), - }.compact - image_alt_text = block.image.alt || "" %> @@ -31,8 +27,11 @@ <%= content_tag("div", class: hero_textbox_classes) do %> <% block.hero_content.each_with_index do |subblock, index| %> <% - options = { margin_bottom: (0 if block.hero_content.length - 1 == index) } - options.merge!(hero_heading_options) if subblock.type == "heading" + is_final_subblock = block.hero_content.length - 1 == index + options = { + margin_bottom: (0 if is_final_subblock), + font_size: ("l" if block.theme == "default" && subblock.type == "heading"), + }.compact %> <%= render_block(subblock, options:) %> <% end %> From d44c6871255fad8224ccb5705d637d8e94202391 Mon Sep 17 00:00:00 2001 From: Richard Towers Date: Tue, 3 Dec 2024 10:27:11 +0000 Subject: [PATCH 7/7] Remove options from featured block We're not actually using the featured block, and it's not totally clear that it should have big headings. So let's simplify the code and remove the option. --- app/views/landing_page/blocks/_featured.html.erb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/views/landing_page/blocks/_featured.html.erb b/app/views/landing_page/blocks/_featured.html.erb index 1a0bacd6db..b0f2604d7f 100644 --- a/app/views/landing_page/blocks/_featured.html.erb +++ b/app/views/landing_page/blocks/_featured.html.erb @@ -7,11 +7,7 @@
<%= tag.div class: featured_child_content_classes do %> <% block.featured_content.each do |subblock| %> - <% - options = {} - options[:font_size] = "l" if subblock.type == "heading" - %> - <%= render_block(subblock, options:) %> + <%= render_block(subblock) %> <% end %> <% end %> <% if block.image %>