From f88db117f18fc417a82101ae503eddb85215c0c3 Mon Sep 17 00:00:00 2001 From: Ben Balter Date: Thu, 6 Apr 2017 19:02:32 -0400 Subject: [PATCH 01/31] convert logic to a drop --- lib/jekyll-seo-tag.rb | 12 ++- lib/jekyll-seo-tag/context.rb | 16 +++ lib/jekyll-seo-tag/drop.rb | 184 +++++++++++++++++++++++++++++++ lib/template.html | 196 +++++++++------------------------- 4 files changed, 255 insertions(+), 153 deletions(-) create mode 100644 lib/jekyll-seo-tag/context.rb create mode 100644 lib/jekyll-seo-tag/drop.rb diff --git a/lib/jekyll-seo-tag.rb b/lib/jekyll-seo-tag.rb index 7d3a736..4707faa 100644 --- a/lib/jekyll-seo-tag.rb +++ b/lib/jekyll-seo-tag.rb @@ -1,4 +1,6 @@ +require "jekyll" require "jekyll-seo-tag/version" +require "jekyll-seo-tag/drop" module Jekyll class SeoTag < Liquid::Tag @@ -36,15 +38,15 @@ module Jekyll def payload { - "page" => context.registers[:page], - "site" => context.registers[:site].site_payload["site"], + "page" => @context.registers[:page], + "site" => @context.registers[:site].site_payload["site"], "paginator" => context["paginator"], - "seo_tag" => options, + "seo_tag" => drop, } end - def title? - @text !~ %r!title=false!i + def drop + Jekyll::SeoTag::Drop.new(@text, @context) end def info diff --git a/lib/jekyll-seo-tag/context.rb b/lib/jekyll-seo-tag/context.rb new file mode 100644 index 0000000..cb1bf1f --- /dev/null +++ b/lib/jekyll-seo-tag/context.rb @@ -0,0 +1,16 @@ +module Jekyll + class SeoTag + # Stubbed LiquidContext to support relative_url and absolute_url helpers + class Context + attr_reader :site + + def initialize(site) + @site = site + end + + def registers + { :site => site } + end + end + end +end diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb new file mode 100644 index 0000000..097c6c7 --- /dev/null +++ b/lib/jekyll-seo-tag/drop.rb @@ -0,0 +1,184 @@ +module Jekyll + class SeoTag + class Drop < Jekyll::Drops::Drop + TITLE_SEPARATOR = " | " + include Jekyll::Filters + include Liquid::StandardFilters + + def initialize(text, context) + @obj = {} + @mutations = {} + @text = text + @context = context + end + + def version + Jekyll::SeoTag::VERSION + end + + # Should the `` tag be generated for this page? + def title? + @text !~ %r!title=false!i && title + end + + def site_title + format_string(site["title"] || site["name"]) + end + + # Page title without site title or description appended + def page_title + format_string(page["title"] || site_title) + end + + # Page title with site title or description appended + def title + if page["title"] && site_title + format_string(page["title"]) + TITLE_SEPARATOR + site_title + elsif site["description"] && site_title + site_title + TITLE_SEPARATOR + format_string(site["description"]) + else + format_string(page["title"]) || site_title + end + end + + def name + if page["seo"] && page["seo"]["name"] + format_string page["seo"]["name"] + elsif homepage_or_about? && site["social"] && site["social"]["name"] + format_string site["social"]["name"] + elsif homepage_or_about? && site_title + format_string site_title + end + end + + def description + format_string(page["description"] || page["excerpt"] || site["description"]) + end + + # Returns a nil or a hash representing the author + # Author name will be pulled from: + # + # 1. The `author` key, if the key is a string + # 2. The first author in the `authors` key + # 3. The `author` key in the site config + # + # If the result from the name search is a string, we'll also check + # to see if the author exists in `site.data.authors` + def author + author = page["author"] + author = page["authors"][0] if author.to_s.empty? && page["authors"] + author = site["author"] if author.to_s.empty? + return if author.to_s.empty? + + if author.is_a?(String) + if site.data["authors"] && site.data["authors"][author] + author = site.data["authors"][author] + else + author = { "name" => author } + end + end + + author["twitter"] ||= author["name"] + author["twitter"].gsub! "@", "" + author.to_liquid + end + + def date_modified + return page["seo"].date_modified if page["seo"] && page["seo"]["date_modified"] + page["last_modified_at"] || page["date"] + end + + def type + if page["seo"] && page["seo"]["type"] + page["seo"]["type"] + elsif homepage_or_about? + "WebSite" + elsif page["date"] + "BlogPosting" + else + "WebPage" + end + end + + def links + if page["seo"] && page["seo"]["links"] + page["seo"]["links"] + elsif homepage_or_about? && site["social"] && site["social"]["links"] + site["social"]["links"] + end + end + + # TODO escape + def logo + return unless site["logo"] + if absolute_url? site["logo"] + site["logo"] + else + absolute_url site["logo"] + end + end + + # Returns nil or a hash representing the page image + # The image hash will always contain a path, pulled from: + # + # 1. The `image` key if it's a string + # 2. The `image.path` key if it's a hash + # 3. The `image.facebook` key + # 4. The `image.twitter` key + # + # The resulting path is always an absolute URL + # TODO escape + def image + return unless image = page["image"] + + image = { "path" => image } if image.is_a?(String) + image["path"] ||= image["facebook"] || image["twitter"] + + unless absolute_url? image["path"] + image["path"] = absolute_url image["path"] + end + + image.to_liquid + end + + def page_lang + page["lang"] || site["lang"] || "en_US" + end + + private + + def page + @page ||= @context.registers[:page].to_liquid + end + + def site + @site ||= @context.registers[:site].site_payload["site"].to_liquid + end + + def homepage_or_about? + ["/", "/index.html", "/about/"].include? page["url"] + end + + def context + @context + end + + def fallback_data + {} + end + + def absolute_url?(string) + string.include? "://" + end + + def format_string(string) + methods = %i(markdownify strip_html normalize_whitespace escape_once) + methods.each do |method| + string = public_send(method, string) + end + + string unless string.empty? + end + end + end +end diff --git a/lib/template.html b/lib/template.html index e3375c2..b28fd7b 100755 --- a/lib/template.html +++ b/lib/template.html @@ -1,143 +1,39 @@ <!-- Begin Jekyll SEO tag v{{ seo_tag.version }} --> - -{% if page.url == "/" or page.url == "/about/" %} - {% assign seo_homepage_or_about = true %} +{% if seo_tag.title? %} + <title>{{ seo_tag.title }} {% endif %} -{% assign seo_site_title = site.title | default: site.name %} -{% assign seo_page_title = page.title | default: seo_site_title %} -{% assign seo_title = page.title | default: seo_site_title %} - -{% if page.title and seo_site_title %} - {% assign seo_title = page.title | append:" | " | append: seo_site_title %} -{% elsif site.description and seo_site_title %} - {% assign seo_title = seo_site_title | append:" | " | append: site.description %} +{% if seo_tag.page_title %} + {% endif %} -{% if page.seo and page.seo.name %} - {% assign seo_name = page.seo.name %} -{% elsif seo_homepage_or_about and site.social and site.social.name %} - {% assign seo_name = site.social.name %} -{% elsif seo_homepage_or_about and seo_site_title %} - {% assign seo_name = seo_site_title %} -{% endif %} -{% if seo_name %} - {% assign seo_name = seo_name | smartify | strip_html | normalize_whitespace | escape_once %} +{% if seo_tag.author.name %} + {% endif %} -{% if seo_title %} - {% assign seo_title = seo_title | smartify | strip_html | normalize_whitespace | escape_once %} + + +{% if seo_tag.description %} + + {% endif %} -{% if seo_site_title %} - {% assign seo_site_title = seo_site_title | smartify | strip_html | normalize_whitespace | escape_once %} -{% endif %} - -{% if seo_page_title %} - {% assign seo_page_title = seo_page_title | smartify | strip_html | normalize_whitespace | escape_once %} -{% endif %} - -{% assign seo_description = page.description | default: page.excerpt | default: site.description %} -{% if seo_description %} - {% assign seo_description = seo_description | markdownify | strip_html | normalize_whitespace | escape_once %} -{% endif %} - -{% assign seo_author = page.author | default: page.authors[0] | default: site.author %} -{% if seo_author %} - {% if seo_author.name %} - {% assign seo_author_name = seo_author.name %} - {% else %} - {% if site.data.authors and site.data.authors[seo_author] %} - {% assign seo_author_name = site.data.authors[seo_author].name %} - {% else %} - {% assign seo_author_name = seo_author %} - {% endif %} - {% endif %} - {% if seo_author.twitter %} - {% assign seo_author_twitter = seo_author.twitter %} - {% else %} - {% if site.data.authors and site.data.authors[seo_author] %} - {% assign seo_author_twitter = site.data.authors[seo_author].twitter %} - {% else %} - {% assign seo_author_twitter = seo_author %} - {% endif %} - {% endif %} - {% assign seo_author_twitter = seo_author_twitter | replace:"@","" %} -{% endif %} - -{% if page.date_modified or page.last_modified_at or page.date %} - {% assign seo_date_modified = page.seo.date_modified | default: page.last_modified_at %} -{% endif %} - -{% if page.seo and page.seo.type %} - {% assign seo_type = page.seo.type %} -{% elsif seo_homepage_or_about %} - {% assign seo_type = "WebSite" %} -{% elsif page.date %} - {% assign seo_type = "BlogPosting" %} -{% else %} - {% assign seo_type = "WebPage" %} -{% endif %} - -{% if page.seo and page.seo.links %} - {% assign seo_links = page.seo.links %} -{% elsif seo_homepage_or_about and site.social and site.social.links %} - {% assign seo_links = site.social.links %} -{% endif %} - -{% if site.logo %} - {% assign seo_site_logo = site.logo %} - {% unless seo_site_logo contains "://" %} - {% assign seo_site_logo = seo_site_logo | absolute_url %} - {% endunless %} - {% assign seo_site_logo = seo_site_logo | escape %} -{% endif %} - -{% if page.image %} - {% assign seo_page_image = page.image.path | default: page.image.facebook | default: page.image.twitter | default: page.image %} - {% unless seo_page_image contains "://" %} - {% assign seo_page_image = seo_page_image | absolute_url %} - {% endunless %} - {% assign seo_page_image = seo_page_image | escape %} -{% endif %} - -{% assign seo_page_lang = page.lang | default: site.lang | default: "en_US" %} - -{% if seo_tag.title and seo_title %} - {{ seo_title }} -{% endif %} - -{% if seo_page_title %} - -{% endif %} - -{% if seo_author_name %} - -{% endif %} - - - -{% if seo_description %} - - -{% endif %} - -{% if page.url %} +{% if site.url %} {% endif %} -{% if seo_site_title %} - +{% if seo_tag.site_title %} + {% endif %} -{% if seo_page_image %} - - {% if page.image.height %} - +{% if seo_tag.image %} + + {% if seo_tag.image.height %} + {% endif %} - {% if page.image.width %} - + {% if seo_tag.image.width %} + {% endif %} {% endif %} @@ -153,8 +49,9 @@ {% endif %} +{{ seo_tag.author | jsonify }} {% if site.twitter %} - {% if seo_page_image or page.image.twitter %} + {% if seo_tag.image %} {% else %} @@ -162,8 +59,8 @@ - {% if seo_author_twitter %} - + {% if seo_tag.author.twitter %} + {% endif %} {% endif %} @@ -185,12 +82,15 @@ {% if site.webmaster_verifications.google %} {% endif %} + {% if site.webmaster_verifications.bing %} {% endif %} + {% if site.webmaster_verifications.alexa %} {% endif %} + {% if site.webmaster_verifications.yandex %} {% endif %} @@ -203,63 +103,63 @@ { "@context": "http://schema.org", -{% if seo_type %} - "@type": {{ seo_type | jsonify }}, +{% if seo_tag.type %} + "@type": {{ seo_tag.type | jsonify }}, {% endif %} -{% if seo_name %} - "name": {{ seo_name | jsonify }}, +{% if seo_tag.name %} + "name": {{ seo_tag.name | jsonify }}, {% endif %} -{% if seo_page_title %} - "headline": {{ seo_page_title | jsonify }}, +{% if seo_tag.page_title %} + "headline": {{ seo_tag.page_title | jsonify }}, {% endif %} -{% if seo_author %} +{% if seo_tag.author %} "author": { "@type": "Person", - "name": {{ seo_author | jsonify }} + "name": {{ seo_tag.author.name | jsonify }} }, {% endif %} -{% if seo_page_image %} - "image": {{ seo_page_image | jsonify }}, +{% if seo_tag.image %} + "image": {{ seo_tag.image.path | jsonify }}, {% endif %} {% if page.date %} "datePublished": {{ page.date | date_to_xmlschema | jsonify }}, {% endif %} -{% if seo_date_modified %} - "dateModified": {{ seo_date_modified | date_to_xmlschema | jsonify }}, +{% if seo_tag.date_modified %} + "dateModified": {{ seo_tag.date_modified | date_to_xmlschema | jsonify }}, {% endif %} -{% if seo_description %} - "description": {{ seo_description | jsonify }}, +{% if seo_tag.description %} + "description": {{ seo_tag.description | jsonify }}, {% endif %} -{% if seo_site_logo %} +{% if seo_tag.logo %} "publisher": { "@type": "Organization", - {% if seo_author %} - "name": {{ seo_author | jsonify }}, + {% if seo_tag.author %} + "name": {{ seo_tag.author.name | jsonify }}, {% endif %} "logo": { "@type": "ImageObject", - "url": {{ seo_site_logo | jsonify }} + "url": {{ seo_tag.logo | jsonify }} } }, {% endif %} -{% if seo_type == "BlogPosting" or seo_type == "CreativeWork"%} +{% if seo_tag.type == "BlogPosting" or seo_tag.type == "CreativeWork"%} "mainEntityOfPage": { "@type": "WebPage", "@id": {{ page.url | replace:'/index.html','/' | absolute_url | jsonify }} }, {% endif %} -{% if seo_links %} - "sameAs": {{ seo_links | jsonify }}, +{% if seo_tag.links %} + "sameAs": {{ seo_tag.links | jsonify }}, {% endif %} "url": {{ page.url | replace:'/index.html','/' | absolute_url | jsonify }} From 25f1401d96d1b0de7b47c9dc7f63b8e4583e5037 Mon Sep 17 00:00:00 2001 From: Ben Balter Date: Thu, 6 Apr 2017 19:04:24 -0400 Subject: [PATCH 02/31] rubocop --- lib/jekyll-seo-tag/drop.rb | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb index 097c6c7..e5da63c 100644 --- a/lib/jekyll-seo-tag/drop.rb +++ b/lib/jekyll-seo-tag/drop.rb @@ -1,12 +1,12 @@ module Jekyll class SeoTag class Drop < Jekyll::Drops::Drop - TITLE_SEPARATOR = " | " + TITLE_SEPARATOR = " | ".freeze include Jekyll::Filters include Liquid::StandardFilters def initialize(text, context) - @obj = {} + @obj = {} @mutations = {} @text = text @context = context @@ -71,15 +71,15 @@ module Jekyll return if author.to_s.empty? if author.is_a?(String) - if site.data["authors"] && site.data["authors"][author] - author = site.data["authors"][author] - else - author = { "name" => author } - end + author = if site.data["authors"] && site.data["authors"][author] + site.data["authors"][author] + else + { "name" => author } + end end author["twitter"] ||= author["name"] - author["twitter"].gsub! "@", "" + author["twitter"].delete! "@" author.to_liquid end @@ -108,7 +108,7 @@ module Jekyll end end - # TODO escape + # TODO: escape def logo return unless site["logo"] if absolute_url? site["logo"] @@ -159,9 +159,7 @@ module Jekyll ["/", "/index.html", "/about/"].include? page["url"] end - def context - @context - end + attr_reader :context def fallback_data {} @@ -172,7 +170,7 @@ module Jekyll end def format_string(string) - methods = %i(markdownify strip_html normalize_whitespace escape_once) + methods = %i[markdownify strip_html normalize_whitespace escape_once] methods.each do |method| string = public_send(method, string) end From b9cd5de1d0055cfc37fee4946b53581e13c84716 Mon Sep 17 00:00:00 2001 From: Ben Balter Date: Thu, 6 Apr 2017 19:06:31 -0400 Subject: [PATCH 03/31] remove stubbed context --- lib/jekyll-seo-tag/context.rb | 16 ---------------- 1 file changed, 16 deletions(-) delete mode 100644 lib/jekyll-seo-tag/context.rb diff --git a/lib/jekyll-seo-tag/context.rb b/lib/jekyll-seo-tag/context.rb deleted file mode 100644 index cb1bf1f..0000000 --- a/lib/jekyll-seo-tag/context.rb +++ /dev/null @@ -1,16 +0,0 @@ -module Jekyll - class SeoTag - # Stubbed LiquidContext to support relative_url and absolute_url helpers - class Context - attr_reader :site - - def initialize(site) - @site = site - end - - def registers - { :site => site } - end - end - end -end From 058b337aba813e4b3b7f648f96a922aa42ff2dd8 Mon Sep 17 00:00:00 2001 From: Ben Balter Date: Thu, 6 Apr 2017 19:07:40 -0400 Subject: [PATCH 04/31] escape urls --- lib/jekyll-seo-tag/drop.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb index e5da63c..badc82d 100644 --- a/lib/jekyll-seo-tag/drop.rb +++ b/lib/jekyll-seo-tag/drop.rb @@ -108,13 +108,12 @@ module Jekyll end end - # TODO: escape def logo return unless site["logo"] if absolute_url? site["logo"] - site["logo"] + uri_escape site["logo"] else - absolute_url site["logo"] + uri_escape absolute_url site["logo"] end end @@ -127,7 +126,6 @@ module Jekyll # 4. The `image.twitter` key # # The resulting path is always an absolute URL - # TODO escape def image return unless image = page["image"] @@ -138,6 +136,8 @@ module Jekyll image["path"] = absolute_url image["path"] end + image["path"] = uri_escape image["path"] + image.to_liquid end From 96e9823561b7bc4ea8eca8859601523b48bb15b4 Mon Sep 17 00:00:00 2001 From: Ben Balter Date: Thu, 6 Apr 2017 22:34:11 -0400 Subject: [PATCH 05/31] add tests for drop --- .rspec | 2 +- lib/jekyll-seo-tag/drop.rb | 57 +++++--- lib/template.html | 1 - spec/jekyll_seo_tag/drop_spec.rb | 233 +++++++++++++++++++++++++++++++ spec/jekyll_seo_tag_spec.rb | 1 + 5 files changed, 273 insertions(+), 21 deletions(-) create mode 100644 spec/jekyll_seo_tag/drop_spec.rb diff --git a/.rspec b/.rspec index 8c18f1a..83e16f8 100644 --- a/.rspec +++ b/.rspec @@ -1,2 +1,2 @@ ---format documentation --color +--require spec_helper diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb index badc82d..b9cf031 100644 --- a/lib/jekyll-seo-tag/drop.rb +++ b/lib/jekyll-seo-tag/drop.rb @@ -18,7 +18,8 @@ module Jekyll # Should the `` tag be generated for this page? def title? - @text !~ %r!title=false!i && title + return false unless title + @text !~ %r!title=false!i end def site_title @@ -33,20 +34,21 @@ module Jekyll # Page title with site title or description appended def title if page["title"] && site_title - format_string(page["title"]) + TITLE_SEPARATOR + site_title + page_title + TITLE_SEPARATOR + site_title elsif site["description"] && site_title site_title + TITLE_SEPARATOR + format_string(site["description"]) else - format_string(page["title"]) || site_title + page_title || site_title end end def name - if page["seo"] && page["seo"]["name"] - format_string page["seo"]["name"] - elsif homepage_or_about? && site["social"] && site["social"]["name"] + return format_string(seo_name) if seo_name + return unless homepage_or_about? + + if site["social"] && site["social"]["name"] format_string site["social"]["name"] - elsif homepage_or_about? && site_title + elsif site_title format_string site_title end end @@ -65,18 +67,13 @@ module Jekyll # If the result from the name search is a string, we'll also check # to see if the author exists in `site.data.authors` def author - author = page["author"] - author = page["authors"][0] if author.to_s.empty? && page["authors"] - author = site["author"] if author.to_s.empty? - return if author.to_s.empty? + return if author_string_or_hash.to_s.empty? - if author.is_a?(String) - author = if site.data["authors"] && site.data["authors"][author] - site.data["authors"][author] - else - { "name" => author } - end - end + author = if author_string_or_hash.is_a?(String) + author_hash(author_string_or_hash) + else + author_string_or_hash + end author["twitter"] ||= author["name"] author["twitter"].delete! "@" @@ -127,7 +124,8 @@ module Jekyll # # The resulting path is always an absolute URL def image - return unless image = page["image"] + image = page["image"] + return unless image image = { "path" => image } if image.is_a?(String) image["path"] ||= image["facebook"] || image["twitter"] @@ -177,6 +175,27 @@ module Jekyll string unless string.empty? end + + def author_string_or_hash + author = page["author"] + author = page["authors"][0] if author.to_s.empty? && page["authors"] + author = site["author"] if author.to_s.empty? + author + end + + def author_hash(author_string) + if site.data["authors"] && site.data["authors"][author_string] + hash = site.data["authors"][author_string] + hash["twitter"] ||= author_string + hash + else + { "name" => author_string } + end + end + + def seo_name + page["seo"] && page["seo"]["name"] + end end end end diff --git a/lib/template.html b/lib/template.html index b28fd7b..d05ce92 100755 --- a/lib/template.html +++ b/lib/template.html @@ -49,7 +49,6 @@ <link rel="next" href="{{ paginator.next_page_path | absolute_url }}"> {% endif %} -{{ seo_tag.author | jsonify }} {% if site.twitter %} {% if seo_tag.image %} <meta name="twitter:card" content="summary_large_image" /> diff --git a/spec/jekyll_seo_tag/drop_spec.rb b/spec/jekyll_seo_tag/drop_spec.rb new file mode 100644 index 0000000..ad7e5f6 --- /dev/null +++ b/spec/jekyll_seo_tag/drop_spec.rb @@ -0,0 +1,233 @@ +RSpec.describe Jekyll::SeoTag::Drop do + let(:page) { make_page({ "title" => "page title" }) } + let(:site) { make_site({ "title" => "site title" }) } + let(:context) { make_context(:page => page, :site => site) } + let(:text) { "" } + subject { described_class.new(text, context) } + + it "returns the version" do + expect(subject.version).to eql(Jekyll::SeoTag::VERSION) + end + + context "title?" do + it "knows to include the title" do + expect(subject.title?).to be_truthy + end + + context "with title=false" do + let(:text) { "title=false" } + + it "knows not to include the title" do + expect(subject.title?).to be_falsy + end + end + + context "site title" do + it "knows the site title" do + expect(subject.site_title).to eql("site title") + end + + context "with site.name" do + let(:site) { make_site({ "name" => "site title" }) } + + it "knows the site title" do + expect(subject.site_title).to eql("site title") + end + end + end + + context "page title" do + it "knows the page title" do + expect(subject.page_title).to eql("page title") + end + + context "without a page title" do + let(:page) { make_page } + + it "knows the page title" do + expect(subject.page_title).to eql("site title") + end + end + end + + context "title" do + context "with a page and site title" do + it "builds the title" do + expect(subject.title).to eql("page title | site title") + end + end + + context "with a site description but no page title" do + let(:page) { make_page } + let(:site) do + make_site({ "title" => "site title", "description" => "site description" }) + end + + it "builds the title" do + expect(subject.title).to eql("site title | site description") + end + end + + context "with just a page title" do + let(:site) { make_site } + + it "builds the title" do + expect(subject.title).to eql("page title") + end + end + + context "with just a site title" do + let(:page) { make_page } + + it "builds the title" do + expect(subject.title).to eql("site title") + end + end + end + end + + context "name" do + context "with seo.name" do + let(:page) { make_page({ "seo" => { "name" => "seo name" } }) } + + it "uses the seo name" do + expect(subject.name).to eql("seo name") + end + end + + context "the index" do + let(:page) { make_page({ "permalink" => "/" }) } + + context "with site.social.name" do + let(:site) { make_site({ "social" => { "name" => "social name" } }) } + + it "uses site.social.name" do + expect(subject.name).to eql("social name") + end + end + + it "uses the site title" do + expect(subject.name).to eql("site title") + end + end + + context "description" do + context "with a page description" do + let(:page) { make_page({ "description"=> "page description" }) } + + it "uses the page description" do + expect(subject.description).to eql("page description") + end + end + + context "with a page excerpt" do + let(:page) { make_page({ "description"=> "page excerpt" }) } + + it "uses the page description" do + expect(subject.description).to eql("page excerpt") + end + end + + context "with a site description" do + let(:site) { make_site({ "description"=> "site description" }) } + + it "uses the page description" do + expect(subject.description).to eql("site description") + end + end + end + + context "author" do + let(:page_data) { {} } + let(:page) { make_page(page_data) } + let(:data) { {} } + let(:site) do + site = make_site({ "author" => "author" }) + site.data = data + site + end + + %i[with without].each do |site_data_type| + context "#{site_data_type} site.author data" do + let(:data) do + if site_data_type == :with + { + "authors" => { + "author" => { "name" => "Author" }, + }, + } + else + {} + end + end + + { + :string => { "author" => "author" }, + :array => { "authors" => %w(author author2) }, + :empty_string => { "author" => "" }, + :nil => { "author" => nil }, + :hash => { "author" => { "name" => "author" } }, + }.each do |author_type, data| + context "with author as #{author_type}" do + let(:page_data) { data } + + it "returns a hash" do + expect(subject.author).to be_a(Hash) + end + + it "returns the name" do + if site_data_type == :with && author_type != :hash + expect(subject.author["name"]).to eql("Author") + else + expect(subject.author["name"]).to eql("author") + end + end + + it "returns the twitter handle" do + expect(subject.author["twitter"]).to eql("author") + end + end + end + end + end + + context "twitter" do + let(:page_data) { { "author" => "author" } } + + it "pulls the handle from the author" do + expect(subject.author["twitter"]).to eql("author") + end + + context "with an @" do + let(:page_data) do + { + "author" => { + "name" => "author", + "twitter" => "@twitter", + }, + } + end + + it "strips the @" do + expect(subject.author["twitter"]).to eql("twitter") + end + end + + context "with an explicit handle" do + let(:page_data) do + { + "author" => { + "name" => "author", + "twitter" => "twitter", + }, + } + end + + it "pulls the handle from the hash" do + expect(subject.author["twitter"]).to eql("twitter") + end + end + end + end + end +end diff --git a/spec/jekyll_seo_tag_spec.rb b/spec/jekyll_seo_tag_spec.rb index 546c2c2..9f5d052 100755 --- a/spec/jekyll_seo_tag_spec.rb +++ b/spec/jekyll_seo_tag_spec.rb @@ -403,6 +403,7 @@ EOS context "with the author in site.data.authors" do let(:author_data) { { "benbalter" => { "twitter" => "test" } } } + it "outputs the twitter card" do expected = %r!<meta name="twitter:creator" content="@test" />! expect(output).to match(expected) From 326e35683fde4b9593789aa1eef4181cdd0f1716 Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Fri, 7 Apr 2017 13:39:05 -0400 Subject: [PATCH 06/31] memoize all the things --- lib/jekyll-seo-tag/drop.rb | 120 +++++++++++++++++++++---------------- 1 file changed, 70 insertions(+), 50 deletions(-) diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb index b9cf031..63220cc 100644 --- a/lib/jekyll-seo-tag/drop.rb +++ b/lib/jekyll-seo-tag/drop.rb @@ -23,38 +23,44 @@ module Jekyll end def site_title - format_string(site["title"] || site["name"]) + @site_title ||= format_string(site["title"] || site["name"]) end # Page title without site title or description appended def page_title - format_string(page["title"] || site_title) + @page_title ||= format_string(page["title"] || site_title) end # Page title with site title or description appended def title - if page["title"] && site_title - page_title + TITLE_SEPARATOR + site_title - elsif site["description"] && site_title - site_title + TITLE_SEPARATOR + format_string(site["description"]) - else - page_title || site_title + @title ||= begin + if page["title"] && site_title + page_title + TITLE_SEPARATOR + site_title + elsif site["description"] && site_title + site_title + TITLE_SEPARATOR + format_string(site["description"]) + else + page_title || site_title + end end end def name - return format_string(seo_name) if seo_name - return unless homepage_or_about? + @name ||= begin + return seo_name if seo_name + return unless homepage_or_about? - if site["social"] && site["social"]["name"] - format_string site["social"]["name"] - elsif site_title - format_string site_title + if site["social"] && site["social"]["name"] + format_string site["social"]["name"] + elsif site_title + format_string site_title + end end end def description - format_string(page["description"] || page["excerpt"] || site["description"]) + @description ||= begin + format_string(page["description"] || page["excerpt"] || site["description"]) + end end # Returns a nil or a hash representing the author @@ -67,50 +73,60 @@ module Jekyll # If the result from the name search is a string, we'll also check # to see if the author exists in `site.data.authors` def author - return if author_string_or_hash.to_s.empty? + @author ||= begin + return if author_string_or_hash.to_s.empty? - author = if author_string_or_hash.is_a?(String) - author_hash(author_string_or_hash) - else - author_string_or_hash - end + author = if author_string_or_hash.is_a?(String) + author_hash(author_string_or_hash) + else + author_string_or_hash + end - author["twitter"] ||= author["name"] - author["twitter"].delete! "@" - author.to_liquid + author["twitter"] ||= author["name"] + author["twitter"].delete! "@" + author.to_liquid + end end def date_modified - return page["seo"].date_modified if page["seo"] && page["seo"]["date_modified"] - page["last_modified_at"] || page["date"] + @date_modified ||= begin + return page["seo"].date_modified if page["seo"] && page["seo"]["date_modified"] + page["last_modified_at"] || page["date"] + end end def type - if page["seo"] && page["seo"]["type"] - page["seo"]["type"] - elsif homepage_or_about? - "WebSite" - elsif page["date"] - "BlogPosting" - else - "WebPage" + @type ||= begin + if page["seo"] && page["seo"]["type"] + page["seo"]["type"] + elsif homepage_or_about? + "WebSite" + elsif page["date"] + "BlogPosting" + else + "WebPage" + end end end def links - if page["seo"] && page["seo"]["links"] - page["seo"]["links"] - elsif homepage_or_about? && site["social"] && site["social"]["links"] - site["social"]["links"] + @links ||= begin + if page["seo"] && page["seo"]["links"] + page["seo"]["links"] + elsif homepage_or_about? && site["social"] && site["social"]["links"] + site["social"]["links"] + end end end def logo - return unless site["logo"] - if absolute_url? site["logo"] - uri_escape site["logo"] - else - uri_escape absolute_url site["logo"] + @logo ||= begin + return unless site["logo"] + if absolute_url? site["logo"] + uri_escape site["logo"] + else + uri_escape absolute_url site["logo"] + end end end @@ -124,6 +140,8 @@ module Jekyll # # The resulting path is always an absolute URL def image + return @image if defined?(@image) + image = page["image"] return unless image @@ -136,11 +154,11 @@ module Jekyll image["path"] = uri_escape image["path"] - image.to_liquid + @image = image.to_liquid end def page_lang - page["lang"] || site["lang"] || "en_US" + @page_lang ||= page["lang"] || site["lang"] || "en_US" end private @@ -177,10 +195,12 @@ module Jekyll end def author_string_or_hash - author = page["author"] - author = page["authors"][0] if author.to_s.empty? && page["authors"] - author = site["author"] if author.to_s.empty? - author + @author_string_or_hash ||= begin + author = page["author"] + author = page["authors"][0] if author.to_s.empty? && page["authors"] + author = site["author"] if author.to_s.empty? + author + end end def author_hash(author_string) @@ -194,7 +214,7 @@ module Jekyll end def seo_name - page["seo"] && page["seo"]["name"] + @seo_name ||= format_string(page["seo"]["name"]) if page["seo"] end end end From 44d52ca4b7ac7f58e250561622466c03a0dddd7e Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Fri, 7 Apr 2017 16:20:54 -0400 Subject: [PATCH 07/31] moar tests --- lib/jekyll-seo-tag/drop.rb | 2 +- spec/jekyll_seo_tag/drop_spec.rb | 154 +++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+), 1 deletion(-) diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb index 63220cc..7bb12a1 100644 --- a/lib/jekyll-seo-tag/drop.rb +++ b/lib/jekyll-seo-tag/drop.rb @@ -90,7 +90,7 @@ module Jekyll def date_modified @date_modified ||= begin - return page["seo"].date_modified if page["seo"] && page["seo"]["date_modified"] + return page["seo"]["date_modified"] if page["seo"] && page["seo"]["date_modified"] page["last_modified_at"] || page["date"] end end diff --git a/spec/jekyll_seo_tag/drop_spec.rb b/spec/jekyll_seo_tag/drop_spec.rb index ad7e5f6..3008fb7 100644 --- a/spec/jekyll_seo_tag/drop_spec.rb +++ b/spec/jekyll_seo_tag/drop_spec.rb @@ -230,4 +230,158 @@ RSpec.describe Jekyll::SeoTag::Drop do end end end + + context "date modified" do + context "with seo.date_modified" do + let(:page) { make_page({ "seo" => { "date_modified" => "tuesday" } }) } + + it "uses seo.date_modified" do + expect(subject.date_modified).to eql("tuesday") + end + end + + context "with page.last_modified_at" do + let(:page) { make_page({ "last_modified_at" => "tuesday" }) } + + it "uses page.last_modified_at" do + expect(subject.date_modified).to eql("tuesday") + end + end + + context "date" do + let(:page) { make_page({ "date" => "tuesday" }) } + + it "uses page.date" do + expect(subject.date_modified).to eql("tuesday") + end + end + end + + context "type" do + context "with seo.type set" do + let(:page) { make_page({ "seo" => { "type" => "test" } }) } + + it "uses seo.type" do + expect(subject.type).to eql("test") + end + end + + context "the homepage" do + let(:page) { make_page({ "permalink" => "/" }) } + + it "is a website" do + expect(subject.type).to eql("WebSite") + end + end + + context "the about page" do + let(:page) { make_page({ "permalink" => "/about/" }) } + + it "is a website" do + expect(subject.type).to eql("WebSite") + end + end + + context "a blog post" do + let(:page) { make_page({ "date" => "2017-01-01" }) } + + it "is a blog post" do + expect(subject.type).to eql("BlogPosting") + end + end + + it "is a webpage" do + expect(subject.type).to eql("WebPage") + end + end + + context "links" do + context "with seo.links" do + let(:page) { make_page({ "seo" => { "links" => %w(foo bar) } }) } + + it "uses seo.links" do + expect(subject.links).to eql(%w(foo bar)) + end + end + + context "with site.social.links" do + let(:site) { make_site({ "social" => { "links"=> %w(a b) } }) } + + it "doesn't use site.social.links" do + expect(subject.links).to be_nil + end + + context "the homepage" do + let(:page) { make_page({ "permalink" => "/" }) } + + it "uses site.social.links" do + expect(subject.links).to eql(%w(a b)) + end + end + end + end + + context "logo" do + context "without site.logo" do + it "returns nothing" do + expect(subject.logo).to be_nil + end + end + + context "with an absolute site.logo" do + let(:site) { make_site({ "logo" => "http://example.com/image.png" }) } + + it "uses site.logo" do + expect(subject.logo).to eql("http://example.com/image.png") + end + end + + context "with a relative site.logo" do + let(:site) do + make_site({ + "logo" => "image.png", + "url" => "http://example.com", + }) + end + + it "uses site.logo" do + expect(subject.logo).to eql("http://example.com/image.png") + end + end + + context "with a uri-escaped logo" do + let(:site) { make_site({ "logo" => "some image.png" }) } + + it "escapes the logo" do + expect(subject.logo).to eql("/some%20image.png") + end + end + end + + context "image" do + end + + context "lang" do + context "with page.lang" do + let(:page) { make_page({ "lang" => "en_GB" }) } + + it "uses page.lang" do + expect(subject.page_lang).to eql("en_GB") + end + end + + context "with site.lang" do + let(:site) { make_site({ "lang" => "en_GB" }) } + + it "uses site.lang" do + expect(subject.page_lang).to eql("en_GB") + end + end + + context "with nothing" do + it "defaults" do + expect(subject.page_lang).to eql("en_US") + end + end + end end From efab79681724a93302e2b533ac2af873ddca65ca Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Fri, 7 Apr 2017 16:27:41 -0400 Subject: [PATCH 08/31] add image specs --- lib/jekyll-seo-tag/drop.rb | 2 +- spec/jekyll_seo_tag/drop_spec.rb | 55 ++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb index 7bb12a1..c968ebc 100644 --- a/lib/jekyll-seo-tag/drop.rb +++ b/lib/jekyll-seo-tag/drop.rb @@ -143,7 +143,7 @@ module Jekyll return @image if defined?(@image) image = page["image"] - return unless image + return @image = nil unless image image = { "path" => image } if image.is_a?(String) image["path"] ||= image["facebook"] || image["twitter"] diff --git a/spec/jekyll_seo_tag/drop_spec.rb b/spec/jekyll_seo_tag/drop_spec.rb index 3008fb7..61aa773 100644 --- a/spec/jekyll_seo_tag/drop_spec.rb +++ b/spec/jekyll_seo_tag/drop_spec.rb @@ -359,6 +359,61 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "image" do + let(:page) { make_page({"image" => image })} + + context "with image as a string" do + let(:image) { "image.png" } + + it "returns a hash" do + expect(subject.image).to be_a(Hash) + end + + it "returns the image" do + expect(subject.image["path"]).to eql("/image.png") + end + + context "with site.url" do + let(:site) { make_site({"url" => "http://example.com"})} + + it "makes the path absolute" do + expect(subject.image["path"]).to eql("http://example.com/image.png") + end + end + + context "with a URL-escaped path" do + let(:image) { "some image.png" } + + it "URL-escapes the image" do + expect(subject.image["path"]).to eql("/some%20image.png") + end + end + end + + context "with image as a hash" do + context "with a path" do + let(:image) { { "path" => "image.png" } } + + it "returns the image" do + expect(subject.image["path"]).to eql("/image.png") + end + end + + context "with facebook" do + let(:image) { { "facebook" => "image.png" } } + + it "returns the image" do + expect(subject.image["path"]).to eql("/image.png") + end + end + + context "with twitter" do + let(:image) { { "twitter" => "image.png" } } + + it "returns the image" do + expect(subject.image["path"]).to eql("/image.png") + end + end + end end context "lang" do From 9620da022465a940c9ffa23d9f857e83875e43f3 Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Fri, 7 Apr 2017 16:34:01 -0400 Subject: [PATCH 09/31] simplify tests --- spec/jekyll_seo_tag/drop_spec.rb | 75 +++++++++++++++++--------------- 1 file changed, 39 insertions(+), 36 deletions(-) diff --git a/spec/jekyll_seo_tag/drop_spec.rb b/spec/jekyll_seo_tag/drop_spec.rb index 61aa773..d56926e 100644 --- a/spec/jekyll_seo_tag/drop_spec.rb +++ b/spec/jekyll_seo_tag/drop_spec.rb @@ -1,6 +1,8 @@ RSpec.describe Jekyll::SeoTag::Drop do - let(:page) { make_page({ "title" => "page title" }) } - let(:site) { make_site({ "title" => "site title" }) } + let(:config) { { "title" => "site title" } } + let(:page_meta) { { "title" => "page title" } } + let(:page) { make_page(page_meta) } + let(:site) { make_site(config) } let(:context) { make_context(:page => page, :site => site) } let(:text) { "" } subject { described_class.new(text, context) } @@ -28,7 +30,7 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "with site.name" do - let(:site) { make_site({ "name" => "site title" }) } + let(:config) { { "name" => "site title" } } it "knows the site title" do expect(subject.site_title).to eql("site title") @@ -59,8 +61,8 @@ RSpec.describe Jekyll::SeoTag::Drop do context "with a site description but no page title" do let(:page) { make_page } - let(:site) do - make_site({ "title" => "site title", "description" => "site description" }) + let(:config) do + { "title" => "site title", "description" => "site description" } end it "builds the title" do @@ -88,7 +90,9 @@ RSpec.describe Jekyll::SeoTag::Drop do context "name" do context "with seo.name" do - let(:page) { make_page({ "seo" => { "name" => "seo name" } }) } + let(:page_meta) do + { "seo" => { "name" => "seo name" } } + end it "uses the seo name" do expect(subject.name).to eql("seo name") @@ -96,10 +100,10 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "the index" do - let(:page) { make_page({ "permalink" => "/" }) } + let(:page_meta) { { "permalink" => "/" } } context "with site.social.name" do - let(:site) { make_site({ "social" => { "name" => "social name" } }) } + let(:config) { { "social" => { "name" => "social name" } } } it "uses site.social.name" do expect(subject.name).to eql("social name") @@ -113,7 +117,7 @@ RSpec.describe Jekyll::SeoTag::Drop do context "description" do context "with a page description" do - let(:page) { make_page({ "description"=> "page description" }) } + let(:page_meta) { { "description"=> "page description" } } it "uses the page description" do expect(subject.description).to eql("page description") @@ -121,7 +125,7 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "with a page excerpt" do - let(:page) { make_page({ "description"=> "page excerpt" }) } + let(:page_meta) { { "description"=> "page excerpt" } } it "uses the page description" do expect(subject.description).to eql("page excerpt") @@ -129,7 +133,7 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "with a site description" do - let(:site) { make_site({ "description"=> "site description" }) } + let(:config) { { "description"=> "site description" } } it "uses the page description" do expect(subject.description).to eql("site description") @@ -138,11 +142,10 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "author" do - let(:page_data) { {} } - let(:page) { make_page(page_data) } let(:data) { {} } + let(:config) { { "author" => "author" } } let(:site) do - site = make_site({ "author" => "author" }) + site = make_site(config) site.data = data site end @@ -169,7 +172,7 @@ RSpec.describe Jekyll::SeoTag::Drop do :hash => { "author" => { "name" => "author" } }, }.each do |author_type, data| context "with author as #{author_type}" do - let(:page_data) { data } + let(:page_meta) { data } it "returns a hash" do expect(subject.author).to be_a(Hash) @@ -192,14 +195,14 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "twitter" do - let(:page_data) { { "author" => "author" } } + let(:page_meta) { { "author" => "author" } } it "pulls the handle from the author" do expect(subject.author["twitter"]).to eql("author") end context "with an @" do - let(:page_data) do + let(:page_meta) do { "author" => { "name" => "author", @@ -214,7 +217,7 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "with an explicit handle" do - let(:page_data) do + let(:page_meta) do { "author" => { "name" => "author", @@ -233,7 +236,7 @@ RSpec.describe Jekyll::SeoTag::Drop do context "date modified" do context "with seo.date_modified" do - let(:page) { make_page({ "seo" => { "date_modified" => "tuesday" } }) } + let(:page_meta) { { "seo" => { "date_modified" => "tuesday" } } } it "uses seo.date_modified" do expect(subject.date_modified).to eql("tuesday") @@ -241,7 +244,7 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "with page.last_modified_at" do - let(:page) { make_page({ "last_modified_at" => "tuesday" }) } + let(:page_meta) { { "last_modified_at" => "tuesday" } } it "uses page.last_modified_at" do expect(subject.date_modified).to eql("tuesday") @@ -249,7 +252,7 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "date" do - let(:page) { make_page({ "date" => "tuesday" }) } + let(:page_meta) { { "date" => "tuesday" } } it "uses page.date" do expect(subject.date_modified).to eql("tuesday") @@ -259,7 +262,7 @@ RSpec.describe Jekyll::SeoTag::Drop do context "type" do context "with seo.type set" do - let(:page) { make_page({ "seo" => { "type" => "test" } }) } + let(:page_meta) { { "seo" => { "type" => "test" } } } it "uses seo.type" do expect(subject.type).to eql("test") @@ -267,7 +270,7 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "the homepage" do - let(:page) { make_page({ "permalink" => "/" }) } + let(:page_meta) { { "permalink" => "/" } } it "is a website" do expect(subject.type).to eql("WebSite") @@ -283,7 +286,7 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "a blog post" do - let(:page) { make_page({ "date" => "2017-01-01" }) } + let(:page_meta) { { "date" => "2017-01-01" } } it "is a blog post" do expect(subject.type).to eql("BlogPosting") @@ -297,7 +300,7 @@ RSpec.describe Jekyll::SeoTag::Drop do context "links" do context "with seo.links" do - let(:page) { make_page({ "seo" => { "links" => %w(foo bar) } }) } + let(:page_meta) { { "seo" => { "links" => %w(foo bar) } } } it "uses seo.links" do expect(subject.links).to eql(%w(foo bar)) @@ -305,14 +308,14 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "with site.social.links" do - let(:site) { make_site({ "social" => { "links"=> %w(a b) } }) } + let(:config) { { "social" => { "links"=> %w(a b) } } } it "doesn't use site.social.links" do expect(subject.links).to be_nil end context "the homepage" do - let(:page) { make_page({ "permalink" => "/" }) } + let(:page_meta) { { "permalink" => "/" } } it "uses site.social.links" do expect(subject.links).to eql(%w(a b)) @@ -329,7 +332,7 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "with an absolute site.logo" do - let(:site) { make_site({ "logo" => "http://example.com/image.png" }) } + let(:config) { { "logo" => "http://example.com/image.png" } } it "uses site.logo" do expect(subject.logo).to eql("http://example.com/image.png") @@ -337,11 +340,11 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "with a relative site.logo" do - let(:site) do - make_site({ + let(:config) do + { "logo" => "image.png", "url" => "http://example.com", - }) + } end it "uses site.logo" do @@ -350,7 +353,7 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "with a uri-escaped logo" do - let(:site) { make_site({ "logo" => "some image.png" }) } + let(:config) { { "logo" => "some image.png" } } it "escapes the logo" do expect(subject.logo).to eql("/some%20image.png") @@ -359,7 +362,7 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "image" do - let(:page) { make_page({"image" => image })} + let(:page_meta) { { "image" => image } } context "with image as a string" do let(:image) { "image.png" } @@ -373,7 +376,7 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "with site.url" do - let(:site) { make_site({"url" => "http://example.com"})} + let(:config) { { "url" => "http://example.com" } } it "makes the path absolute" do expect(subject.image["path"]).to eql("http://example.com/image.png") @@ -418,7 +421,7 @@ RSpec.describe Jekyll::SeoTag::Drop do context "lang" do context "with page.lang" do - let(:page) { make_page({ "lang" => "en_GB" }) } + let(:page_meta) { { "lang" => "en_GB" } } it "uses page.lang" do expect(subject.page_lang).to eql("en_GB") @@ -426,7 +429,7 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "with site.lang" do - let(:site) { make_site({ "lang" => "en_GB" }) } + let(:config) { { "lang" => "en_GB" } } it "uses site.lang" do expect(subject.page_lang).to eql("en_GB") From 3797b19a7df53e8ece22a114b2e9b7cb90193467 Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Fri, 7 Apr 2017 16:35:20 -0400 Subject: [PATCH 10/31] make rubocop happy --- lib/jekyll-seo-tag/drop.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb index c968ebc..082a745 100644 --- a/lib/jekyll-seo-tag/drop.rb +++ b/lib/jekyll-seo-tag/drop.rb @@ -90,7 +90,9 @@ module Jekyll def date_modified @date_modified ||= begin - return page["seo"]["date_modified"] if page["seo"] && page["seo"]["date_modified"] + if page["seo"] && page["seo"]["date_modified"] + return page["seo"]["date_modified"] + end page["last_modified_at"] || page["date"] end end From 7885669ee6fa49a5dd15cabc4120f660c46664a1 Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Fri, 7 Apr 2017 16:37:21 -0400 Subject: [PATCH 11/31] memoize drop --- lib/jekyll-seo-tag.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/jekyll-seo-tag.rb b/lib/jekyll-seo-tag.rb index 4707faa..eb1c938 100644 --- a/lib/jekyll-seo-tag.rb +++ b/lib/jekyll-seo-tag.rb @@ -46,7 +46,7 @@ module Jekyll end def drop - Jekyll::SeoTag::Drop.new(@text, @context) + @drop ||= Jekyll::SeoTag::Drop.new(@text, @context) end def info From 0fd1bdcdfc42266a079f227841fe1deb958ef55d Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Fri, 7 Apr 2017 16:51:37 -0400 Subject: [PATCH 12/31] ensure to_h doesnt cause things to explode --- lib/jekyll-seo-tag/drop.rb | 10 +++++++--- spec/jekyll_seo_tag/drop_spec.rb | 8 ++++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb index 082a745..3910407 100644 --- a/lib/jekyll-seo-tag/drop.rb +++ b/lib/jekyll-seo-tag/drop.rb @@ -2,8 +2,12 @@ module Jekyll class SeoTag class Drop < Jekyll::Drops::Drop TITLE_SEPARATOR = " | ".freeze - include Jekyll::Filters - include Liquid::StandardFilters + INCLUDES = [Jekyll::Filters, Liquid::StandardFilters].freeze + + INCLUDES.each do |klass| + include klass + klass.instance_methods.each { |method| private method } + end def initialize(text, context) @obj = {} @@ -190,7 +194,7 @@ module Jekyll def format_string(string) methods = %i[markdownify strip_html normalize_whitespace escape_once] methods.each do |method| - string = public_send(method, string) + string = send(method, string) end string unless string.empty? diff --git a/spec/jekyll_seo_tag/drop_spec.rb b/spec/jekyll_seo_tag/drop_spec.rb index d56926e..34ef79c 100644 --- a/spec/jekyll_seo_tag/drop_spec.rb +++ b/spec/jekyll_seo_tag/drop_spec.rb @@ -7,6 +7,14 @@ RSpec.describe Jekyll::SeoTag::Drop do let(:text) { "" } subject { described_class.new(text, context) } + # Drop includes liquid filters which expect arguments + # By default, in drops, `to_h` will call each public method with no arugments + # Here, that would cause the filters to explode. This test ensures that all + # public methods don't explode when called without arguments. Don't explode. + it "doesn't blow up on to_h" do + expect { subject.to_h }.to_not raise_error + end + it "returns the version" do expect(subject.version).to eql(Jekyll::SeoTag::VERSION) end From dea21f0f8d7723d342ac2c54aca5923547409ff3 Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Fri, 7 Apr 2017 17:01:20 -0400 Subject: [PATCH 13/31] move filters to their own class --- lib/jekyll-seo-tag.rb | 4 +++- lib/jekyll-seo-tag/drop.rb | 20 +++++++++----------- lib/jekyll-seo-tag/filters.rb | 12 ++++++++++++ 3 files changed, 24 insertions(+), 12 deletions(-) create mode 100644 lib/jekyll-seo-tag/filters.rb diff --git a/lib/jekyll-seo-tag.rb b/lib/jekyll-seo-tag.rb index eb1c938..689e5b1 100644 --- a/lib/jekyll-seo-tag.rb +++ b/lib/jekyll-seo-tag.rb @@ -1,9 +1,11 @@ require "jekyll" require "jekyll-seo-tag/version" -require "jekyll-seo-tag/drop" module Jekyll class SeoTag < Liquid::Tag + autoload :Drop, "jekyll-seo-tag/drop" + autoload :Filters, "jekyll-seo-tag/filters" + attr_accessor :context # Matches all whitespace that follows either diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb index 3910407..a2f7714 100644 --- a/lib/jekyll-seo-tag/drop.rb +++ b/lib/jekyll-seo-tag/drop.rb @@ -2,12 +2,6 @@ module Jekyll class SeoTag class Drop < Jekyll::Drops::Drop TITLE_SEPARATOR = " | ".freeze - INCLUDES = [Jekyll::Filters, Liquid::StandardFilters].freeze - - INCLUDES.each do |klass| - include klass - klass.instance_methods.each { |method| private method } - end def initialize(text, context) @obj = {} @@ -129,9 +123,9 @@ module Jekyll @logo ||= begin return unless site["logo"] if absolute_url? site["logo"] - uri_escape site["logo"] + filters.uri_escape site["logo"] else - uri_escape absolute_url site["logo"] + filters.uri_escape filters.absolute_url site["logo"] end end end @@ -155,10 +149,10 @@ module Jekyll image["path"] ||= image["facebook"] || image["twitter"] unless absolute_url? image["path"] - image["path"] = absolute_url image["path"] + image["path"] = filters.absolute_url image["path"] end - image["path"] = uri_escape image["path"] + image["path"] = filters.uri_escape image["path"] @image = image.to_liquid end @@ -169,6 +163,10 @@ module Jekyll private + def filters + @filters ||= Jekyll::SeoTag::Filters.new(@context) + end + def page @page ||= @context.registers[:page].to_liquid end @@ -194,7 +192,7 @@ module Jekyll def format_string(string) methods = %i[markdownify strip_html normalize_whitespace escape_once] methods.each do |method| - string = send(method, string) + string = filters.public_send(method, string) end string unless string.empty? diff --git a/lib/jekyll-seo-tag/filters.rb b/lib/jekyll-seo-tag/filters.rb new file mode 100644 index 0000000..2874aed --- /dev/null +++ b/lib/jekyll-seo-tag/filters.rb @@ -0,0 +1,12 @@ +module Jekyll + class SeoTag + class Filters + include Jekyll::Filters + include Liquid::StandardFilters + + def initialize(context) + @context = context + end + end + end +end From fa44b312802b771a26be5df7c5bed484e2b91810 Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Fri, 7 Apr 2017 17:06:03 -0400 Subject: [PATCH 14/31] add spec for filters class --- lib/jekyll-seo-tag/drop.rb | 6 ++++-- spec/jekyll_seo_tag/filters_spec.rb | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 spec/jekyll_seo_tag/filters_spec.rb diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb index a2f7714..34cec8d 100644 --- a/lib/jekyll-seo-tag/drop.rb +++ b/lib/jekyll-seo-tag/drop.rb @@ -2,6 +2,9 @@ module Jekyll class SeoTag class Drop < Jekyll::Drops::Drop TITLE_SEPARATOR = " | ".freeze + FORMAT_STRING_METHODS = %i[ + markdownify strip_html normalize_whitespace escape_once + ].freeze def initialize(text, context) @obj = {} @@ -190,8 +193,7 @@ module Jekyll end def format_string(string) - methods = %i[markdownify strip_html normalize_whitespace escape_once] - methods.each do |method| + FORMAT_STRING_METHODS.each do |method| string = filters.public_send(method, string) end diff --git a/spec/jekyll_seo_tag/filters_spec.rb b/spec/jekyll_seo_tag/filters_spec.rb new file mode 100644 index 0000000..c44d5cb --- /dev/null +++ b/spec/jekyll_seo_tag/filters_spec.rb @@ -0,0 +1,18 @@ +RSpec.describe Jekyll::SeoTag::Filters do + let(:page) { make_page } + let(:site) { make_site } + let(:context) { make_context(:page => page, :site => site) } + subject { described_class.new(context) } + + it "stores the context" do + expect(subject.instance_variable_get("@context")).to be_a(Liquid::Context) + end + + it "exposes jekyll filters" do + expect(subject).to respond_to(:markdownify) + end + + it "exposes liquid standard filters" do + expect(subject).to respond_to(:normalize_whitespace) + end +end From 337bffa3bc5a84bdc81b99046800912470dcd32f Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Fri, 7 Apr 2017 17:10:13 -0400 Subject: [PATCH 15/31] use attr_accessor --- lib/jekyll-seo-tag.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/jekyll-seo-tag.rb b/lib/jekyll-seo-tag.rb index 689e5b1..9f9b7e2 100644 --- a/lib/jekyll-seo-tag.rb +++ b/lib/jekyll-seo-tag.rb @@ -40,8 +40,8 @@ module Jekyll def payload { - "page" => @context.registers[:page], - "site" => @context.registers[:site].site_payload["site"], + "page" => context.registers[:page], + "site" => context.registers[:site].site_payload["site"], "paginator" => context["paginator"], "seo_tag" => drop, } From 66aff234ca4badbb2e040fbaad21dd90afd13558 Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Sat, 8 Apr 2017 14:20:21 -0400 Subject: [PATCH 16/31] use ruby to generate json-ld --- lib/jekyll-seo-tag.rb | 1 + lib/jekyll-seo-tag/drop.rb | 16 +++-- lib/jekyll-seo-tag/json_ld.rb | 76 ++++++++++++++++++++ lib/template.html | 69 +----------------- spec/jekyll_seo_tag/drop_spec.rb | 12 ++-- spec/jekyll_seo_tag/json_ld_spec.rb | 106 ++++++++++++++++++++++++++++ spec/jekyll_seo_tag_spec.rb | 6 +- 7 files changed, 206 insertions(+), 80 deletions(-) create mode 100644 lib/jekyll-seo-tag/json_ld.rb create mode 100644 spec/jekyll_seo_tag/json_ld_spec.rb diff --git a/lib/jekyll-seo-tag.rb b/lib/jekyll-seo-tag.rb index 9f9b7e2..6d3e4b8 100644 --- a/lib/jekyll-seo-tag.rb +++ b/lib/jekyll-seo-tag.rb @@ -3,6 +3,7 @@ require "jekyll-seo-tag/version" module Jekyll class SeoTag < Liquid::Tag + autoload :JSONLD, "jekyll-seo-tag/json_ld" autoload :Drop, "jekyll-seo-tag/drop" autoload :Filters, "jekyll-seo-tag/filters" diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb index 34cec8d..8e07d71 100644 --- a/lib/jekyll-seo-tag/drop.rb +++ b/lib/jekyll-seo-tag/drop.rb @@ -1,6 +1,8 @@ module Jekyll class SeoTag class Drop < Jekyll::Drops::Drop + include Jekyll::SeoTag::JSONLD + TITLE_SEPARATOR = " | ".freeze FORMAT_STRING_METHODS = %i[ markdownify strip_html normalize_whitespace escape_once @@ -91,10 +93,12 @@ module Jekyll def date_modified @date_modified ||= begin - if page["seo"] && page["seo"]["date_modified"] - return page["seo"]["date_modified"] - end - page["last_modified_at"] || page["date"] + date = if page["seo"] && page["seo"]["date_modified"] + page["seo"]["date_modified"] + else + page["last_modified_at"] || page["date"] + end + filters.date_to_xmlschema(date) if date end end @@ -164,6 +168,10 @@ module Jekyll @page_lang ||= page["lang"] || site["lang"] || "en_US" end + def canonical_url + @canonical_url ||= filters.absolute_url(page["url"]).gsub(%r!/index\.html$!, "/") + end + private def filters diff --git a/lib/jekyll-seo-tag/json_ld.rb b/lib/jekyll-seo-tag/json_ld.rb new file mode 100644 index 0000000..f4eb9fd --- /dev/null +++ b/lib/jekyll-seo-tag/json_ld.rb @@ -0,0 +1,76 @@ +module Jekyll + class SeoTag + module JSONLD + + # A hash of instance methods => key in resulting JSON-LD hash + METHODS_KEYS = { + :json_context => "@context", + :type => "@type", + :name => "name", + :page_title => "headline", + :json_author => "author", + :image_path => "image", + :date_modified => "dateModified", + :description => "description", + :publisher => "publisher", + :main_entity => "mainEntityOfPage", + :links => "sameAs", + :canonical_url => "url", + }.freeze + + def json_ld + @json_ld ||= begin + output = {} + METHODS_KEYS.each do |method, key| + value = send(method) + output[key] = value unless value.nil? + end + output + end + end + + private + + def json_context + "http://schema.org" + end + + def json_author + return unless author + { + "@type" => "Person", + "name" => author["name"], + } + end + + def image_path + image["path"] if image + end + + def date + filters.date_to_xmlschema(page["date"]) if page["date"] + end + + def publisher + return unless logo + output = { + "@type" => "Organization", + "logo" => { + "@type" => "ImageObject", + "url" => logo, + }, + } + output["name"] = author["name"] if author + output + end + + def main_entity + return unless %w(BlogPosting CreativeWork).include?(type) + { + "@type" => "WebPage", + "@id" => canonical_url, + } + end + end + end +end diff --git a/lib/template.html b/lib/template.html index d05ce92..8a6a7aa 100755 --- a/lib/template.html +++ b/lib/template.html @@ -19,8 +19,8 @@ {% endif %} {% if site.url %} - <link rel="canonical" href="{{ page.url | replace:'/index.html','/' | absolute_url }}" /> - <meta property="og:url" content="{{ page.url | replace:'/index.html','/' | absolute_url }}" /> + <link rel="canonical" href="{{ seo_tag.canonical_url }}" /> + <meta property="og:url" content="{{ seo_tag.canonical_url }}" /> {% endif %} {% if seo_tag.site_title %} @@ -99,70 +99,7 @@ <script type="application/ld+json"> - { - "@context": "http://schema.org", - -{% if seo_tag.type %} - "@type": {{ seo_tag.type | jsonify }}, -{% endif %} - -{% if seo_tag.name %} - "name": {{ seo_tag.name | jsonify }}, -{% endif %} - -{% if seo_tag.page_title %} - "headline": {{ seo_tag.page_title | jsonify }}, -{% endif %} - -{% if seo_tag.author %} - "author": { - "@type": "Person", - "name": {{ seo_tag.author.name | jsonify }} - }, -{% endif %} - -{% if seo_tag.image %} - "image": {{ seo_tag.image.path | jsonify }}, -{% endif %} - -{% if page.date %} - "datePublished": {{ page.date | date_to_xmlschema | jsonify }}, -{% endif %} - -{% if seo_tag.date_modified %} - "dateModified": {{ seo_tag.date_modified | date_to_xmlschema | jsonify }}, -{% endif %} - -{% if seo_tag.description %} - "description": {{ seo_tag.description | jsonify }}, -{% endif %} - -{% if seo_tag.logo %} - "publisher": { - "@type": "Organization", - {% if seo_tag.author %} - "name": {{ seo_tag.author.name | jsonify }}, - {% endif %} - "logo": { - "@type": "ImageObject", - "url": {{ seo_tag.logo | jsonify }} - } - }, -{% endif %} - -{% if seo_tag.type == "BlogPosting" or seo_tag.type == "CreativeWork"%} - "mainEntityOfPage": { - "@type": "WebPage", - "@id": {{ page.url | replace:'/index.html','/' | absolute_url | jsonify }} - }, -{% endif %} - -{% if seo_tag.links %} - "sameAs": {{ seo_tag.links | jsonify }}, -{% endif %} - - "url": {{ page.url | replace:'/index.html','/' | absolute_url | jsonify }} - } + {{ seo_tag.json_ld | jsonify }} </script> <!-- End Jekyll SEO tag --> diff --git a/spec/jekyll_seo_tag/drop_spec.rb b/spec/jekyll_seo_tag/drop_spec.rb index 34ef79c..2c8008b 100644 --- a/spec/jekyll_seo_tag/drop_spec.rb +++ b/spec/jekyll_seo_tag/drop_spec.rb @@ -244,26 +244,26 @@ RSpec.describe Jekyll::SeoTag::Drop do context "date modified" do context "with seo.date_modified" do - let(:page_meta) { { "seo" => { "date_modified" => "tuesday" } } } + let(:page_meta) { { "seo" => { "date_modified" => "2017-01-01" } } } it "uses seo.date_modified" do - expect(subject.date_modified).to eql("tuesday") + expect(subject.date_modified).to eql("2017-01-01T00:00:00-05:00") end end context "with page.last_modified_at" do - let(:page_meta) { { "last_modified_at" => "tuesday" } } + let(:page_meta) { { "last_modified_at" => "2017-01-01" } } it "uses page.last_modified_at" do - expect(subject.date_modified).to eql("tuesday") + expect(subject.date_modified).to eql("2017-01-01T00:00:00-05:00") end end context "date" do - let(:page_meta) { { "date" => "tuesday" } } + let(:page_meta) { { "date" => "2017-01-01" } } it "uses page.date" do - expect(subject.date_modified).to eql("tuesday") + expect(subject.date_modified).to eql("2017-01-01T00:00:00-05:00") end end end diff --git a/spec/jekyll_seo_tag/json_ld_spec.rb b/spec/jekyll_seo_tag/json_ld_spec.rb new file mode 100644 index 0000000..68e043f --- /dev/null +++ b/spec/jekyll_seo_tag/json_ld_spec.rb @@ -0,0 +1,106 @@ +RSpec.describe Jekyll::SeoTag::JSONLD do + let(:metadata) do + { + "title" => "title", + "author" => "author", + "image" => "image", + "date" => "2017-01-01", + "description" => "description", + "seo" => { + "name" => "seo name", + "date_modified" => "2017-01-01", + "links" => %w(a b), + }, + } + end + let(:config) do + { + "logo" => "logo", + } + end + let(:page) { make_page(metadata) } + let(:site) { make_site(config) } + let(:context) { make_context(:page => page, :site => site) } + subject { Jekyll::SeoTag::Drop.new("", context).json_ld } + + it "returns the context" do + expect(subject).to have_key("@context") + expect(subject["@context"]).to eql("http://schema.org") + end + + it "returns the type" do + expect(subject).to have_key("@type") + expect(subject["@type"]).to eql("BlogPosting") + end + + it "returns the name" do + expect(subject).to have_key("name") + expect(subject["name"]).to eql("seo name") + end + + it "returns the headline" do + expect(subject).to have_key("headline") + expect(subject["headline"]).to eql("title") + end + + it "returns the author" do + expect(subject).to have_key("author") + expect(subject["author"]).to be_a(Hash) + expect(subject["author"]).to have_key("@type") + expect(subject["author"]["@type"]).to eql("Person") + expect(subject["author"]).to have_key("name") + expect(subject["author"]["name"]).to eql("author") + end + + it "returns the image" do + expect(subject).to have_key("image") + expect(subject["image"]).to eql("/image") + end + + it "returns the dateModified" do + expect(subject).to have_key("dateModified") + expect(subject["dateModified"]).to eql("2017-01-01T00:00:00-05:00") + end + + it "returns the description" do + expect(subject).to have_key("description") + expect(subject["description"]).to eql("description") + end + + it "returns the publisher" do + expect(subject).to have_key("publisher") + + publisher = subject["publisher"] + expect(publisher).to be_a(Hash) + + expect(publisher).to have_key("@type") + expect(publisher["@type"]).to eql("Organization") + expect(publisher).to have_key("logo") + + logo = publisher["logo"] + expect(logo).to have_key("@type") + expect(logo["@type"]).to eql("ImageObject") + expect(logo).to have_key("url") + expect(logo["url"]).to eql("/logo") + end + + it "returns the main entity of page" do + expect(subject).to have_key("mainEntityOfPage") + expect(subject["mainEntityOfPage"]).to be_a(Hash) + expect(subject["mainEntityOfPage"]).to have_key("@type") + expect(subject["mainEntityOfPage"]["@type"]).to eql("WebPage") + expect(subject["mainEntityOfPage"]).to have_key("@id") + expect(subject["mainEntityOfPage"]["@id"]).to eql("/page.html") + end + + it "returns sameAs" do + expect(subject).to have_key("sameAs") + expect(subject["sameAs"]).to be_a(Array) + expect(subject["sameAs"]).to eql(%w(a b)) + end + + it "returns the url" do + expect(subject).to have_key("url") + expect(subject["url"]).to eql("/page.html") + end +end diff --git a/spec/jekyll_seo_tag_spec.rb b/spec/jekyll_seo_tag_spec.rb index 9f5d052..bcc5d76 100755 --- a/spec/jekyll_seo_tag_spec.rb +++ b/spec/jekyll_seo_tag_spec.rb @@ -331,10 +331,8 @@ EOS end it "minifies JSON-LD" do - expected = <<-EOS -{"@context": "http://schema.org", -"@type": "BlogPosting", -"headline": "post", + expected = <<-EOS.strip +{"@context":"http://schema.org","@type":"BlogPosting","headline":"post", EOS expect(output).to match(expected) end From 8a3cd3f9e357dae139e9e6c4598edae6af0db328 Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Sat, 8 Apr 2017 14:27:45 -0400 Subject: [PATCH 17/31] specify timezone in tests --- lib/template.html | 1 - spec/jekyll_seo_tag/drop_spec.rb | 2 ++ spec/jekyll_seo_tag/json_ld_spec.rb | 3 ++- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/template.html b/lib/template.html index 8a6a7aa..d51169b 100755 --- a/lib/template.html +++ b/lib/template.html @@ -97,7 +97,6 @@ <meta name="google-site-verification" content="{{ site.google_site_verification }}" /> {% endif %} - <script type="application/ld+json"> {{ seo_tag.json_ld | jsonify }} </script> diff --git a/spec/jekyll_seo_tag/drop_spec.rb b/spec/jekyll_seo_tag/drop_spec.rb index 2c8008b..4ae3085 100644 --- a/spec/jekyll_seo_tag/drop_spec.rb +++ b/spec/jekyll_seo_tag/drop_spec.rb @@ -243,6 +243,8 @@ RSpec.describe Jekyll::SeoTag::Drop do end context "date modified" do + let(:config) { { "timezone" => "America/New_York" } } + context "with seo.date_modified" do let(:page_meta) { { "seo" => { "date_modified" => "2017-01-01" } } } diff --git a/spec/jekyll_seo_tag/json_ld_spec.rb b/spec/jekyll_seo_tag/json_ld_spec.rb index 68e043f..c5f3b9b 100644 --- a/spec/jekyll_seo_tag/json_ld_spec.rb +++ b/spec/jekyll_seo_tag/json_ld_spec.rb @@ -15,7 +15,8 @@ RSpec.describe Jekyll::SeoTag::JSONLD do end let(:config) do { - "logo" => "logo", + "logo" => "logo", + "timezone" => "America/New_York", } end let(:page) { make_page(metadata) } From 6eaae965f85bf2900c07e2f76f466e0f52b46c92 Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Sat, 8 Apr 2017 14:29:24 -0400 Subject: [PATCH 18/31] silence Jekyll output --- spec/jekyll_seo_tag/drop_spec.rb | 4 ++++ spec/jekyll_seo_tag/filters_spec.rb | 4 ++++ spec/jekyll_seo_tag/json_ld_spec.rb | 4 ++++ spec/jekyll_seo_tag_spec.rb | 4 +--- 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/spec/jekyll_seo_tag/drop_spec.rb b/spec/jekyll_seo_tag/drop_spec.rb index 4ae3085..d9ab962 100644 --- a/spec/jekyll_seo_tag/drop_spec.rb +++ b/spec/jekyll_seo_tag/drop_spec.rb @@ -7,6 +7,10 @@ RSpec.describe Jekyll::SeoTag::Drop do let(:text) { "" } subject { described_class.new(text, context) } + before do + Jekyll.logger.log_level = :error + end + # Drop includes liquid filters which expect arguments # By default, in drops, `to_h` will call each public method with no arugments # Here, that would cause the filters to explode. This test ensures that all diff --git a/spec/jekyll_seo_tag/filters_spec.rb b/spec/jekyll_seo_tag/filters_spec.rb index c44d5cb..c3cf1c2 100644 --- a/spec/jekyll_seo_tag/filters_spec.rb +++ b/spec/jekyll_seo_tag/filters_spec.rb @@ -4,6 +4,10 @@ RSpec.describe Jekyll::SeoTag::Filters do let(:context) { make_context(:page => page, :site => site) } subject { described_class.new(context) } + before do + Jekyll.logger.log_level = :error + end + it "stores the context" do expect(subject.instance_variable_get("@context")).to be_a(Liquid::Context) end diff --git a/spec/jekyll_seo_tag/json_ld_spec.rb b/spec/jekyll_seo_tag/json_ld_spec.rb index c5f3b9b..58a6d9f 100644 --- a/spec/jekyll_seo_tag/json_ld_spec.rb +++ b/spec/jekyll_seo_tag/json_ld_spec.rb @@ -1,4 +1,8 @@ RSpec.describe Jekyll::SeoTag::JSONLD do + before do + Jekyll.logger.log_level = :error + end + let(:metadata) do { "title" => "title", diff --git a/spec/jekyll_seo_tag_spec.rb b/spec/jekyll_seo_tag_spec.rb index bcc5d76..a0f3ca8 100755 --- a/spec/jekyll_seo_tag_spec.rb +++ b/spec/jekyll_seo_tag_spec.rb @@ -1,6 +1,4 @@ -require "spec_helper" - -describe Jekyll::SeoTag do +RSpec.describe Jekyll::SeoTag do let(:page) { make_page } let(:site) { make_site } let(:post) { make_post } From 9c06b710e61e9e69fa12508719f0daf47d398e7f Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Sat, 8 Apr 2017 14:29:53 -0400 Subject: [PATCH 19/31] call it an integration spec --- ...{jekyll_seo_tag_spec.rb => jekyll_seo_tag_integration_spec.rb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename spec/{jekyll_seo_tag_spec.rb => jekyll_seo_tag_integration_spec.rb} (100%) diff --git a/spec/jekyll_seo_tag_spec.rb b/spec/jekyll_seo_tag_integration_spec.rb similarity index 100% rename from spec/jekyll_seo_tag_spec.rb rename to spec/jekyll_seo_tag_integration_spec.rb From de12942f36d6122c2988153bc1c4b9d4a21802e6 Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Sat, 8 Apr 2017 14:31:59 -0400 Subject: [PATCH 20/31] remove unused date method --- lib/jekyll-seo-tag/json_ld.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/jekyll-seo-tag/json_ld.rb b/lib/jekyll-seo-tag/json_ld.rb index f4eb9fd..f33c59a 100644 --- a/lib/jekyll-seo-tag/json_ld.rb +++ b/lib/jekyll-seo-tag/json_ld.rb @@ -47,10 +47,6 @@ module Jekyll image["path"] if image end - def date - filters.date_to_xmlschema(page["date"]) if page["date"] - end - def publisher return unless logo output = { From d2f053dd4353561bf4b94963ae9704f0a78a0174 Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Mon, 10 Apr 2017 09:58:41 -0500 Subject: [PATCH 21/31] add datePublished to JSON-LD schema --- lib/jekyll-seo-tag/drop.rb | 4 ++++ lib/jekyll-seo-tag/json_ld.rb | 25 +++++++++++++------------ spec/jekyll_seo_tag/drop_spec.rb | 9 +++++++++ spec/jekyll_seo_tag/json_ld_spec.rb | 9 +++++++-- 4 files changed, 33 insertions(+), 14 deletions(-) diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb index 8e07d71..d5784d0 100644 --- a/lib/jekyll-seo-tag/drop.rb +++ b/lib/jekyll-seo-tag/drop.rb @@ -102,6 +102,10 @@ module Jekyll end end + def date_published + @date_published ||= filters.date_to_xmlschema(page["date"]) if page["date"] + end + def type @type ||= begin if page["seo"] && page["seo"]["type"] diff --git a/lib/jekyll-seo-tag/json_ld.rb b/lib/jekyll-seo-tag/json_ld.rb index f33c59a..4439039 100644 --- a/lib/jekyll-seo-tag/json_ld.rb +++ b/lib/jekyll-seo-tag/json_ld.rb @@ -4,18 +4,19 @@ module Jekyll # A hash of instance methods => key in resulting JSON-LD hash METHODS_KEYS = { - :json_context => "@context", - :type => "@type", - :name => "name", - :page_title => "headline", - :json_author => "author", - :image_path => "image", - :date_modified => "dateModified", - :description => "description", - :publisher => "publisher", - :main_entity => "mainEntityOfPage", - :links => "sameAs", - :canonical_url => "url", + :json_context => "@context", + :type => "@type", + :name => "name", + :page_title => "headline", + :json_author => "author", + :image_path => "image", + :date_published => "datePublished", + :date_modified => "dateModified", + :description => "description", + :publisher => "publisher", + :main_entity => "mainEntityOfPage", + :links => "sameAs", + :canonical_url => "url", }.freeze def json_ld diff --git a/spec/jekyll_seo_tag/drop_spec.rb b/spec/jekyll_seo_tag/drop_spec.rb index d9ab962..678cfb4 100644 --- a/spec/jekyll_seo_tag/drop_spec.rb +++ b/spec/jekyll_seo_tag/drop_spec.rb @@ -246,6 +246,15 @@ RSpec.describe Jekyll::SeoTag::Drop do end end + context "date published" do + let(:config) { { "timezone" => "America/New_York" } } + let(:page_meta) { { "date" => "2017-01-01" } } + + it "uses page.date" do + expect(subject.date_published).to eql("2017-01-01T00:00:00-05:00") + end + end + context "date modified" do let(:config) { { "timezone" => "America/New_York" } } diff --git a/spec/jekyll_seo_tag/json_ld_spec.rb b/spec/jekyll_seo_tag/json_ld_spec.rb index 58a6d9f..1b86945 100644 --- a/spec/jekyll_seo_tag/json_ld_spec.rb +++ b/spec/jekyll_seo_tag/json_ld_spec.rb @@ -12,7 +12,7 @@ RSpec.describe Jekyll::SeoTag::JSONLD do "description" => "description", "seo" => { "name" => "seo name", - "date_modified" => "2017-01-01", + "date_modified" => "2017-01-02", "links" => %w(a b), }, } @@ -62,9 +62,14 @@ RSpec.describe Jekyll::SeoTag::JSONLD do expect(subject["image"]).to eql("/image") end + it "returns the datePublished" do + expect(subject).to have_key("datePublished") + expect(subject["datePublished"]).to eql("2017-01-01T00:00:00-05:00") + end + it "returns the dateModified" do expect(subject).to have_key("dateModified") - expect(subject["dateModified"]).to eql("2017-01-01T00:00:00-05:00") + expect(subject["dateModified"]).to eql("2017-01-02T00:00:00-05:00") end it "returns the description" do From 791db5107cd570095e6722b08fd93ef0485ee0f0 Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Tue, 11 Apr 2017 14:45:49 -0500 Subject: [PATCH 22/31] check author passed as a hash in json-ld --- spec/jekyll_seo_tag/json_ld_spec.rb | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/spec/jekyll_seo_tag/json_ld_spec.rb b/spec/jekyll_seo_tag/json_ld_spec.rb index 1b86945..e920e8f 100644 --- a/spec/jekyll_seo_tag/json_ld_spec.rb +++ b/spec/jekyll_seo_tag/json_ld_spec.rb @@ -3,10 +3,11 @@ RSpec.describe Jekyll::SeoTag::JSONLD do Jekyll.logger.log_level = :error end + let(:author) { "author" } let(:metadata) do { "title" => "title", - "author" => "author", + "author" => author, "image" => "image", "date" => "2017-01-01", "description" => "description", @@ -48,13 +49,25 @@ RSpec.describe Jekyll::SeoTag::JSONLD do expect(subject["headline"]).to eql("title") end - it "returns the author" do - expect(subject).to have_key("author") - expect(subject["author"]).to be_a(Hash) - expect(subject["author"]).to have_key("@type") - expect(subject["author"]["@type"]).to eql("Person") - expect(subject["author"]).to have_key("name") - expect(subject["author"]["name"]).to eql("author") + context "author" do + { + "string" => "author", + "hash" => { "name" => "author" }, + }.each do |key, value| + context "when passed as a #{key}" do + let(:author) { value } + + it "returns the author" do + expect(subject).to have_key("author") + expect(subject["author"]).to be_a(Hash) + expect(subject["author"]).to have_key("@type") + expect(subject["author"]["@type"]).to eql("Person") + expect(subject["author"]).to have_key("name") + expect(subject["author"]["name"]).to be_a(String) + expect(subject["author"]["name"]).to eql("author") + end + end + end end it "returns the image" do From 993eafce00fb1f0665db2f10a1b042fb98db1480 Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Tue, 11 Apr 2017 14:54:21 -0500 Subject: [PATCH 23/31] memoize title? --- lib/jekyll-seo-tag/drop.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb index d5784d0..97f599e 100644 --- a/lib/jekyll-seo-tag/drop.rb +++ b/lib/jekyll-seo-tag/drop.rb @@ -22,7 +22,8 @@ module Jekyll # Should the `<title>` tag be generated for this page? def title? return false unless title - @text !~ %r!title=false!i + return @display_title if defined?(@display_title) + @display_title = (@text !~ %r!title=false!i) end def site_title From ee1b03cd0a942160de22cd0d52cd0cc444f58753 Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Tue, 11 Apr 2017 14:58:10 -0500 Subject: [PATCH 24/31] memoize name --- lib/jekyll-seo-tag/drop.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb index 97f599e..6c9eaa7 100644 --- a/lib/jekyll-seo-tag/drop.rb +++ b/lib/jekyll-seo-tag/drop.rb @@ -49,16 +49,16 @@ module Jekyll end def name - @name ||= begin - return seo_name if seo_name - return unless homepage_or_about? - - if site["social"] && site["social"]["name"] - format_string site["social"]["name"] - elsif site_title - format_string site_title - end - end + return @name if defined?(@name) + @name = if seo_name + seo_name + elsif !homepage_or_about? + nil + elsif site["social"] && site["social"]["name"] + format_string site["social"]["name"] + elsif site_title + format_string site_title + end end def description From 0a70e8c172c7b609b3b74a357ee7864c60ee618f Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Tue, 11 Apr 2017 14:59:19 -0500 Subject: [PATCH 25/31] remove stray begin block --- lib/jekyll-seo-tag/drop.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb index 6c9eaa7..e157160 100644 --- a/lib/jekyll-seo-tag/drop.rb +++ b/lib/jekyll-seo-tag/drop.rb @@ -62,9 +62,9 @@ module Jekyll end def description - @description ||= begin - format_string(page["description"] || page["excerpt"] || site["description"]) - end + @description ||= format_string( + page["description"] || page["excerpt"] || site["description"] + ) end # Returns a nil or a hash representing the author From ab916c974e886a01ebfb40cb5781f5a1adedd3c4 Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Tue, 11 Apr 2017 14:59:45 -0500 Subject: [PATCH 26/31] memoize fallback_data --- lib/jekyll-seo-tag/drop.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb index e157160..a0116ca 100644 --- a/lib/jekyll-seo-tag/drop.rb +++ b/lib/jekyll-seo-tag/drop.rb @@ -198,7 +198,7 @@ module Jekyll attr_reader :context def fallback_data - {} + @fallback_data ||= {} end def absolute_url?(string) From cfe7e56cf860ea8b671e4930d69ede8d84ff2bfa Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Tue, 11 Apr 2017 15:13:24 -0500 Subject: [PATCH 27/31] better homepage_or_about detection --- lib/jekyll-seo-tag/drop.rb | 3 ++- spec/jekyll_seo_tag/drop_spec.rb | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb index a0116ca..b876180 100644 --- a/lib/jekyll-seo-tag/drop.rb +++ b/lib/jekyll-seo-tag/drop.rb @@ -7,6 +7,7 @@ module Jekyll FORMAT_STRING_METHODS = %i[ markdownify strip_html normalize_whitespace escape_once ].freeze + HOMEPAGE_OR_ABOUT_REGEX = %r!^/(about/)?(index.html?)?$! def initialize(text, context) @obj = {} @@ -192,7 +193,7 @@ module Jekyll end def homepage_or_about? - ["/", "/index.html", "/about/"].include? page["url"] + page["url"] =~ HOMEPAGE_OR_ABOUT_REGEX end attr_reader :context diff --git a/spec/jekyll_seo_tag/drop_spec.rb b/spec/jekyll_seo_tag/drop_spec.rb index 678cfb4..ab0b852 100644 --- a/spec/jekyll_seo_tag/drop_spec.rb +++ b/spec/jekyll_seo_tag/drop_spec.rb @@ -465,4 +465,27 @@ RSpec.describe Jekyll::SeoTag::Drop do end end end + + context "homepage_or_about?" do + [ + "/", "/index.html", "index.html", "/index.htm", + "/about/", "/about/index.html", + ].each do |permalink| + context "when passed '#{permalink}' as a permalink" do + let(:page_meta) { { "permalink" => permalink } } + + it "knows it's the home or about page" do + expect(subject.send(:homepage_or_about?)).to be_truthy + end + end + end + + context "a random URL" do + let(:page_meta) { { "permalink" => "/about-foo/" } } + + it "knows it's not the home or about page" do + expect(subject.send(:homepage_or_about?)).to be_falsy + end + end + end end From 8cb5953ea45e677f66f3ae000576f9de7537aec0 Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Tue, 11 Apr 2017 15:15:24 -0500 Subject: [PATCH 28/31] use addressible to determine absolute urls --- lib/jekyll-seo-tag/drop.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb index b876180..7cc5a6a 100644 --- a/lib/jekyll-seo-tag/drop.rb +++ b/lib/jekyll-seo-tag/drop.rb @@ -203,7 +203,7 @@ module Jekyll end def absolute_url?(string) - string.include? "://" + Addressable::URI.parse(string).absolute? end def format_string(string) From 650fe5c1fdbb00f78e7879ff95b6c98957ad32a7 Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Tue, 11 Apr 2017 15:19:17 -0500 Subject: [PATCH 29/31] use reduce to format strings --- lib/jekyll-seo-tag/drop.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/jekyll-seo-tag/drop.rb b/lib/jekyll-seo-tag/drop.rb index 7cc5a6a..e997536 100644 --- a/lib/jekyll-seo-tag/drop.rb +++ b/lib/jekyll-seo-tag/drop.rb @@ -207,8 +207,8 @@ module Jekyll end def format_string(string) - FORMAT_STRING_METHODS.each do |method| - string = filters.public_send(method, string) + string = FORMAT_STRING_METHODS.reduce(string) do |memo, method| + filters.public_send(method, memo) end string unless string.empty? From 0f8edbce4a72d509365ee707d7fdc64a06f2bd78 Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Tue, 11 Apr 2017 15:29:43 -0500 Subject: [PATCH 30/31] add test for specifying image height and width --- spec/jekyll_seo_tag/drop_spec.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/spec/jekyll_seo_tag/drop_spec.rb b/spec/jekyll_seo_tag/drop_spec.rb index ab0b852..d61e7b2 100644 --- a/spec/jekyll_seo_tag/drop_spec.rb +++ b/spec/jekyll_seo_tag/drop_spec.rb @@ -439,6 +439,15 @@ RSpec.describe Jekyll::SeoTag::Drop do expect(subject.image["path"]).to eql("/image.png") end end + + context "with height and width" do + let(:image) { { "path" => "image.png", "height" => 5, "width" => 10 } } + + it "returns the height and width" do + expect(subject.image["height"]).to eql(5) + expect(subject.image["width"]).to eql(10) + end + end end end From bc0f45ae64baf500c5b288f3ec21de8a2a2c14e0 Mon Sep 17 00:00:00 2001 From: Ben Balter <ben.balter@github.com> Date: Tue, 11 Apr 2017 16:33:13 -0500 Subject: [PATCH 31/31] output image as an imageObject if we have additional image properties --- lib/jekyll-seo-tag/json_ld.rb | 12 ++++++++--- spec/jekyll_seo_tag/json_ld_spec.rb | 31 +++++++++++++++++++++++++---- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/lib/jekyll-seo-tag/json_ld.rb b/lib/jekyll-seo-tag/json_ld.rb index 4439039..318685f 100644 --- a/lib/jekyll-seo-tag/json_ld.rb +++ b/lib/jekyll-seo-tag/json_ld.rb @@ -9,7 +9,7 @@ module Jekyll :name => "name", :page_title => "headline", :json_author => "author", - :image_path => "image", + :json_image => "image", :date_published => "datePublished", :date_modified => "dateModified", :description => "description", @@ -44,8 +44,14 @@ module Jekyll } end - def image_path - image["path"] if image + def json_image + return unless image + return image["path"] if image.length == 1 + + hash = image.dup + hash["url"] = hash.delete("path") + hash["@type"] = "imageObject" + hash end def publisher diff --git a/spec/jekyll_seo_tag/json_ld_spec.rb b/spec/jekyll_seo_tag/json_ld_spec.rb index e920e8f..88b987a 100644 --- a/spec/jekyll_seo_tag/json_ld_spec.rb +++ b/spec/jekyll_seo_tag/json_ld_spec.rb @@ -4,11 +4,12 @@ RSpec.describe Jekyll::SeoTag::JSONLD do end let(:author) { "author" } + let(:image) { "image" } let(:metadata) do { "title" => "title", "author" => author, - "image" => "image", + "image" => image, "date" => "2017-01-01", "description" => "description", "seo" => { @@ -70,9 +71,31 @@ RSpec.describe Jekyll::SeoTag::JSONLD do end end - it "returns the image" do - expect(subject).to have_key("image") - expect(subject["image"]).to eql("/image") + context "image" do + context "with image as a string" do + let(:image) { "image" } + + it "returns the image as a string" do + expect(subject).to have_key("image") + expect(subject["image"]).to be_a(String) + expect(subject["image"]).to eql("/image") + end + end + + context "with image as a hash" do + let(:image) { { "path" => "image", "height" => 5, "width" => 10 } } + + it "returns the image as a hash" do + expect(subject).to have_key("image") + expect(subject["image"]).to be_a(Hash) + expect(subject["image"]).to eql({ + "@type" => "imageObject", + "url" => "/image", + "height" => 5, + "width" => 10, + }) + end + end end it "returns the datePublished" do