maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Hero Billboard Placement (#19467)

Browse source 
* init

* add option to swagger docs

* in_house validation

* filtered ad quesry changes

* styling and prioritizing home hero banner over campaign banner

* rubocop

* typo

* broken spec

* more spec fixes

* found the spec break culprit

* derp

* Update app/views/shared/_display_ad.html.erb

Co-authored-by: Ridhwana <Ridhwana.Khan16@gmail.com>

* Update app/controllers/stories_controller.rb

Co-authored-by: Ridhwana <Ridhwana.Khan16@gmail.com>

* remove immediate return

* styling feedback and moving banner to articles index

* remove nil

* updated styling

* add signed in bool

* add feature flag

* remove feature flag data scripts

---------

Co-authored-by: Ridhwana <Ridhwana.Khan16@gmail.com>
This commit is contained in:
Lawrence 2023-05-18 12:37:32 -05:00 • committed by GitHub
parent 8b24e4c255
commit 1b32e9a8e3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 115 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

42
app/assets/stylesheets/billboards.scss
View file

@ -40,6 +40,48 @@
display: block;
}
.crayons-card.hero-billboard .ltag_cta {
display: block;
}
.hero-billboard {
margin: var(--su-2) 0 0;
h1,
h2,
h3,
p {
text-align: center;
}
@media (max-width: $breakpoint-s) {
display: block;
}
@media (min-width: $breakpoint-m) {
margin: var(--su-2) var(--su-2) 0;
}
@media (min-width: $breakpoint-l) {
margin: var(--su-4) var(--su-4) 0;
}
@media (min-width: $breakpoint-xl) {
max-width: var(--site-width);
margin: var(--su-4) var(--su-4) 0;
}
@media (min-width: 1284px) {
max-width: var(--site-width);
margin: var(--su-4) auto 0;
}
}
@media (min-width: $breakpoint-s) {
.hero-billboard h1 {
font-size: var(--fs-3xl);
}
}
// these are inline CTA's
//refers to the below the comments billboard on the article page
//refers to the feed/stories where we use story

5
app/controllers/stories_controller.rb
View file

@ -48,6 +48,10 @@ class StoriesController < ApplicationController
private
def assign_hero_banner
@hero_display_ad = DisplayAd.for_display(area: "home_hero", user_signed_in: user_signed_in?)
end
def assign_hero_html
return if Campaign.current.hero_html_variant_name.blank?
@ -125,6 +129,7 @@ class StoriesController < ApplicationController
def handle_base_index
@home_page = true
assign_feed_stories unless user_signed_in? # Feed fetched async for signed-in users
assign_hero_banner
assign_hero_html
assign_podcasts
get_latest_campaign_articles if Campaign.current.show_in_sidebar?

10
app/models/display_ad.rb
View file

@ -6,7 +6,7 @@ class DisplayAd < ApplicationRecord
belongs_to :audience_segment, optional: true
# rubocop:disable Layout/LineLength
ALLOWED_PLACEMENT_AREAS = %w[sidebar_left sidebar_left_2 sidebar_right feed_first feed_second feed_third post_sidebar post_comments].freeze
ALLOWED_PLACEMENT_AREAS = %w[sidebar_left sidebar_left_2 sidebar_right feed_first feed_second feed_third home_hero post_sidebar post_comments].freeze
# rubocop:enable Layout/LineLength
ALLOWED_PLACEMENT_AREAS_HUMAN_READABLE = ["Sidebar Left (First Position)",
"Sidebar Left (Second Position)",
@ -14,6 +14,7 @@ class DisplayAd < ApplicationRecord
"Home Feed First",
"Home Feed Second",
"Home Feed Third",
"Home Hero",
"Sidebar Right (Individual Post)",
"Below the comment section"].freeze
@ -35,6 +36,7 @@ class DisplayAd < ApplicationRecord
validates :body_markdown, presence: true
validates :organization, presence: true, if: :community?
validate :validate_tag
validate :validate_in_house_hero_ads
before_save :process_markdown
after_save :generate_display_ad_name
@ -96,6 +98,12 @@ class DisplayAd < ApplicationRecord
validate_tag_name(tag_list)
end
def validate_in_house_hero_ads
return unless placement_area == "home_hero" && type_of != "in_house"
errors.add(:type_of, "must be in_house if display ad is a Home Hero")
end
def audience_segment_type=(type)
self.audience_segment = if type.blank?
nil

3
app/views/articles/index.html.erb
View file

@ -26,6 +26,9 @@
<%= javascript_packs_with_chunks_tag "homePage", defer: true %>
<% cache(release_adjusted_cache_key("main-stories-index-#{params}-#{user_signed_in?}"), expires_in: 90.seconds) do %>
<% if @hero_display_ad && FeatureFlag.enabled?(:hero_billboard) %>
<%= render partial: "shared/display_ad", locals: { display_ad: @hero_display_ad, data_context_type: DisplayAdEvent::CONTEXT_TYPE_HOME } %>
<% end %>
<div class="crayons-layout crayons-layout--3-cols crayons-layout--3-cols--drop-right-left" id="index-container"
data-params="<%= params.merge(sort_by: "hotness_score", sort_direction: "desc").to_json(only: %i[tag username q sort_by sort_direction]) %>" data-which="<%= @list_of %>"
data-tag=""

6
app/views/layouts/application.html.erb
View file

@ -55,8 +55,7 @@
data-deployed-at="<%= j(ForemInstance.deployed_at) %>"
data-latest-commit-id="<%= j(ForemInstance.latest_commit_id) %>"
data-ga-tracking="<%= j(Settings::General.ga_tracking_id) %>"
data-ga4-tracking-id="<%= j(Settings::General.ga_analytics_4_id) %>"
>
data-ga4-tracking-id="<%= j(Settings::General.ga_analytics_4_id) %>">
<%# Repeat of stylesheets in <head> to fix rendering glitch: https://github.com/forem/forem/issues/12377 %>
<%= render "layouts/styles", qualifier: "secondary" %>
<div id="body-styles">
@ -89,7 +88,8 @@
<% end %>
<div id="page-content-inner">
<div id="page-route-change" class="screen-reader-only" aria-live="polite" aria-atomic="true"></div>
<% if @hero_html %>
<% if @hero_html && @hero_display_ad.blank? %>
<aside id="hero-html-wrapper" aria-label="Campaign banner" class="overflow-x-hidden" data-name="<%= @hero_area.name %>">
<script>
// Make sure html variant element has id hero-html-wrapper and data-name as its name

12
app/views/shared/_display_ad.html.erb
View file

@ -16,6 +16,18 @@
</div>
</div>
</div>
<% elsif display_ad.placement_area == "home_hero" %>
<div class="crayons-card crayons-card--secondary crayons-sponsorship crayons-story__display-ad hero-billboard"
data-display-unit data-id="<%= display_ad.id %>"
data-category-click="<%= DisplayAdEvent::CATEGORY_CLICK %>"
data-category-impression="<%= DisplayAdEvent::CATEGORY_IMPRESSION %>"
data-context-type="<%= data_context_type %>"
data-type-of="<%= display_ad.type_of %>">
<%= render partial: "shared/display_ad_header", locals: { display_ad: display_ad } %>
<div class="p-1 text-styles text-styles--display-ad">
<%= display_ad.processed_html.html_safe %>
</div>
</div>
<% else %>
<div class="crayons-card crayons-card--secondary crayons-sponsorship billboard"
data-display-unit data-id="<%= display_ad.id %>"

14
spec/models/display_ad_spec.rb
View file

@ -30,6 +30,20 @@ RSpec.describe DisplayAd do
expect(display_ad).to be_valid
end
it "allows home_hero with in_house" do
display_ad.placement_area = "home_hero"
display_ad.type_of = "in_house"
expect(display_ad).to be_valid
end
it "does not allow home_hero with community" do
display_ad.placement_area = "home_hero"
display_ad.type_of = "community"
expect(display_ad).not_to be_valid
expect(display_ad.errors[:type_of])
.to include("must be in_house if display ad is a Home Hero")
end
it "disallows unacceptable placement_area" do
display_ad.placement_area = "tsdsdsdds"
expect(display_ad).not_to be_valid

26
spec/queries/display_ads/filtered_ads_query_spec.rb
View file

@ -76,22 +76,22 @@ RSpec.describe DisplayAds::FilteredAdsQuery, type: :query do
end
context "when considering article_exclude_ids" do
let!(:exclude_article1) { create_display_ad exclude_article_ids: "11,12" }
let!(:exclude_article2) { create_display_ad exclude_article_ids: "12,13" }
let!(:ex_article1) { create_display_ad exclude_article_ids: "11,12" }
let!(:another_ex_article2) { create_display_ad exclude_article_ids: "12,13" }
let!(:no_excludes) { create_display_ad }
it "will show display ads that exclude articles appropriately" do
filtered = filter_ads article_id: 11
expect(filtered).to contain_exactly(exclude_article2, no_excludes)
expect(filtered).to contain_exactly(another_ex_article2, no_excludes)
filtered = filter_ads article_id: 12
expect(filtered).to contain_exactly(no_excludes)
filtered = filter_ads article_id: 13
expect(filtered).to contain_exactly(exclude_article1, no_excludes)
expect(filtered).to contain_exactly(ex_article1, no_excludes)
filtered = filter_ads article_id: 14
expect(filtered).to contain_exactly(exclude_article1, exclude_article2, no_excludes)
expect(filtered).to contain_exactly(ex_article1, another_ex_article2, no_excludes)
end
end
@ -153,4 +153,20 @@ RSpec.describe DisplayAds::FilteredAdsQuery, type: :query do
expect(filtered).not_to include(other_community)
end
end
context "when considering home hero ads" do
let!(:in_house_ad) { create_display_ad placement_area: "home_hero", type_of: :in_house }
let(:organization) { create(:organization) }
let(:other_org) { create(:organization) }
let!(:community_ad) { create_display_ad organization_id: organization.id, type_of: :community }
let!(:other_community) { create_display_ad organization_id: other_org.id, type_of: :community }
it "always shows home hero ads only" do
filtered = filter_ads(area: "home_hero")
expect(filtered).to contain_exactly(in_house_ad)
expect(filtered).not_to include(other_community)
expect(filtered).not_to include(community_ad)
end
end
end

2
spec/system/organization/user_views_an_organization_spec.rb
View file

@ -13,7 +13,7 @@ RSpec.describe "Organization index" do
before { visit "/#{organization.slug}" }
it "shows the header", js: true do
within("h1") { expect(page).to have_content(organization.name) }
within("h1.crayons-title") { expect(page).to have_content(organization.name) }
within("div.profile-header__actions") do
expect(page).to have_button(I18n.t("core.follow"))
end

4
spec/system/search/search_title_spec.rb
View file

@ -12,7 +12,7 @@ RSpec.describe "Search page title" do
visit "/search?q=helloworld"
expect(page).to have_title("Search Results for helloworld - DEV(local)")
expect(page.find("h1")).to have_content("Search results for helloworld")
expect(page.find("h1.crayons-title")).to have_content("Search results for helloworld")
end
end
@ -21,7 +21,7 @@ RSpec.describe "Search page title" do
visit "/search"
expect(page).to have_title("Search Results - DEV(local)")
expect(page.find("h1")).to have_content("Search results")
expect(page.find("h1.crayons-title")).to have_content("Search results")
end
end
end

4
spec/system/user/view_user_index_spec.rb
View file

@ -15,7 +15,7 @@ RSpec.describe "User index" do
context "when 1 article" do
it "shows header", :aggregate_failures, js: true do
within("h1") { expect(page).to have_content(user.name) }
within("h1.crayons-title") { expect(page).to have_content(user.name) }
within(".profile-header__actions") do
expect(page).to have_button(I18n.t("core.follow"))
end
@ -99,7 +99,7 @@ RSpec.describe "User index" do
context "when user is logged in" do
it "shows_header", :aggregate_failures, js: true do
within("h1") { expect(page).to have_content(user.name) }
within("h1.crayons-title") { expect(page).to have_content(user.name) }
within(".profile-header__actions") do
expect(page).to have_button(I18n.t("core.edit_profile"))
end

1
swagger/v1/api_v1.json
View file

@ -3522,6 +3522,7 @@
"feed_first",
"feed_second",
"feed_third",
"home_hero",
"post_sidebar",
"post_comments"
],