From 69878e27a66b04f76ef373b01292ac44ed472ac3 Mon Sep 17 00:00:00 2001 From: Richard Towers Date: Thu, 11 Mar 2021 14:33:18 +0000 Subject: [PATCH 1/6] Add tabindex and focus states to code blocks This is required for accessibility reasons. Because some code blocks have scrollbars they need to be focusable using a keyboard. I've copied the styles from the design system's documentation (see the code blocks in the HTML / Nunjucks tabs here: https://design-system.service.gov.uk/components/button/) The implementation is simple when syntax highlighting is not enabled, because we can simply implement `block_code` to surround the code with the markup we want. When syntax highlighting is enabled it's a bit more complicated. Middleman already `include`s its own implementation of `block_code`, which renders the source code highlighted with spans. This method doesn't look like it would be easy to customise, so instead I've resorted to post-processing the HTML (i.e. replacing the attributes on the `
` tag using a regex).

Testing this was a bit tricky, because you need to monkey patch the
class to include the `block_code` method. I had to clone the class and
patch the clone to do avoid affecting other tests.
---
 .../modules/_technical-documentation.scss     |  6 ++
 .../tech_docs_html_renderer.rb                | 18 +++++
 .../tech_docs_html_renderer_spec.rb           | 68 +++++++++++++++++++
 3 files changed, 92 insertions(+)

diff --git a/lib/assets/stylesheets/modules/_technical-documentation.scss b/lib/assets/stylesheets/modules/_technical-documentation.scss
index cc76de64..bae69750 100644
--- a/lib/assets/stylesheets/modules/_technical-documentation.scss
+++ b/lib/assets/stylesheets/modules/_technical-documentation.scss
@@ -123,6 +123,12 @@
     overflow: auto;
     position: relative;
     border: 1px solid $code-02;
+
+    &:focus {
+      padding: 13px;
+      border: $govuk-focus-width solid $govuk-focus-text-colour;
+      outline: $govuk-focus-width solid $govuk-focus-colour;
+    }
   }
 
   pre code {
diff --git a/lib/govuk_tech_docs/tech_docs_html_renderer.rb b/lib/govuk_tech_docs/tech_docs_html_renderer.rb
index 8b191ccc..86416a14 100644
--- a/lib/govuk_tech_docs/tech_docs_html_renderer.rb
+++ b/lib/govuk_tech_docs/tech_docs_html_renderer.rb
@@ -74,5 +74,23 @@ def table_row(body)
 
       tr.to_html
     end
+
+    def block_code(text, lang)
+      if defined?(super)
+        # Post-processing the block_code HTML to implement tabbable code blocks.
+        #
+        # Middleman monkey patches the Middleman::Renderers::MiddlemanRedcarpetHTML
+        # to include Middleman::Syntax::RedcarpetCodeRenderer. This defines its own
+        # version of `block_code(text, lang)` which we can call with `super`.
+
+        highlighted_html = super
+        highlighted_html.sub("
#{text}
"
+      end
+    end
   end
 end
diff --git a/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb b/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb
index 3e856bac..81f119fb 100644
--- a/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb
+++ b/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb
@@ -1,3 +1,5 @@
+require "middleman-syntax/extension"
+
 RSpec.describe GovukTechDocs::TechDocsHTMLRenderer do
   let(:app) { double("app") }
   let(:context) { double("context") }
@@ -42,3 +44,69 @@
     end
   end
 end
+
+RSpec.describe "code blocks" do
+  let(:app) { double("app") }
+  let(:context) { double("context") }
+
+  before :each do
+    allow(context).to receive(:app) { app }
+    allow(app).to receive(:api)
+  end
+
+  describe "without syntax highlighting" do
+    let(:processor) {
+      Redcarpet::Markdown.new(GovukTechDocs::TechDocsHTMLRenderer.new(context: context), fenced_code_blocks: true)
+    }
+
+    describe "#render" do
+      it "sets tab index to 0" do
+        output = processor.render <<~MARKDOWN
+          Hello world:
+
+          ```ruby
+          def hello_world
+            puts "hello world"
+          end
+          ```
+        MARKDOWN
+
+        expect(output).to include('
')
+        expect(output).to include("def hello_world")
+        expect(output).to include('puts "hello world"')
+        expect(output).to include("end")
+      end
+    end
+  end
+
+  describe "with syntax highlighting" do
+    let(:processor) {
+      renderer_class = GovukTechDocs::TechDocsHTMLRenderer.clone.tap do |c|
+        c.send :include, Middleman::Syntax::RedcarpetCodeRenderer
+      end
+
+      Redcarpet::Markdown.new(renderer_class.new(context: context), fenced_code_blocks: true)
+    }
+
+    describe "#render" do
+      it "sets tab index to 0" do
+        output = processor.render <<~MARKDOWN
+          Hello world:
+
+          ```ruby
+          def hello_world
+            puts "hello world"
+          end
+          ```
+        MARKDOWN
+
+        expect(output).to include('
')
+        expect(output).to include("def")
+        expect(output).to include("hello_world")
+        expect(output).to include("puts")
+        expect(output).to include('"hello world"')
+        expect(output).to include("end")
+      end
+    end
+  end
+end

From 3271053a6e3fd49357df457d6480139c8316d116 Mon Sep 17 00:00:00 2001
From: Richard Towers 
Date: Thu, 11 Mar 2021 15:51:43 +0000
Subject: [PATCH 2/6] Calculate focus changes to padding and border

This doesn't change the CSS generated, but makes it a bit clearer why
the padding is 13px in the focus state and 15px otherwise. It should
also make it easier to keep in sync with future changes to padding /
borders.
---
 .../stylesheets/modules/_technical-documentation.scss    | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lib/assets/stylesheets/modules/_technical-documentation.scss b/lib/assets/stylesheets/modules/_technical-documentation.scss
index bae69750..d5dff2c5 100644
--- a/lib/assets/stylesheets/modules/_technical-documentation.scss
+++ b/lib/assets/stylesheets/modules/_technical-documentation.scss
@@ -118,14 +118,17 @@
   }
 
   pre {
+    $padding: 15px;
+    $border-width: 1px;
+
     background: $code-00;
-    padding: 15px;
+    padding: $padding;
     overflow: auto;
     position: relative;
-    border: 1px solid $code-02;
+    border: $border-width solid $code-02;
 
     &:focus {
-      padding: 13px;
+      padding: $padding - ($govuk-focus-width - $border-width);
       border: $govuk-focus-width solid $govuk-focus-text-colour;
       outline: $govuk-focus-width solid $govuk-focus-colour;
     }

From 8efa6f940f0fc2512b9522724d3cbed05ba798a0 Mon Sep 17 00:00:00 2001
From: Richard Towers 
Date: Fri, 12 Mar 2021 13:43:48 +0000
Subject: [PATCH 3/6] Improve structure of tests

We don't need to call processor.render in every test, as it's always
called with the same arguments. That means it can be pulled out into a
`let` block.

Moving the code block tests into the top level describe lets us use
described_class in the tests, which cuts down on a bit of syntax noise.

We can get a bit clever by only replacing `processor` and not `output`
in the "with syntax highlighting" case - the test will use the `output`
block from `describe "#render a code block"`, but with the `processor`
from `describe "with syntax highlighting"`
---
 .../tech_docs_html_renderer_spec.rb           | 123 ++++++++----------
 1 file changed, 52 insertions(+), 71 deletions(-)

diff --git a/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb b/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb
index 81f119fb..05efa2f2 100644
--- a/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb
+++ b/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb
@@ -4,108 +4,89 @@
   let(:app) { double("app") }
   let(:context) { double("context") }
   let(:processor) {
+    Redcarpet::Markdown.new(described_class.new(context: context), tables: true, fenced_code_blocks: true)
+  }
+
+  before :each do
     allow(context).to receive(:app) { app }
     allow(app).to receive(:api)
-    Redcarpet::Markdown.new(described_class.new(context: context), tables: true)
-  }
+  end
 
   describe "#render a table" do
-    markdown_table = <<~MARKDOWN
-     |  A   | B |
-     |------|---|
-     |# C   | D |
-     |  E   | F |
-     |# *G* | H |
-    MARKDOWN
+    let(:output) {
+      processor.render <<~MARKDOWN
+        |  A   | B |
+        |------|---|
+        |# C   | D |
+        |  E   | F |
+        |# *G* | H |
+      MARKDOWN
+    }
 
     it "treats cells in the heading row as headings" do
-      output = processor.render markdown_table
-
       expect(output).to include("A")
       expect(output).to include("B")
     end
 
     it "treats cells starting with # as row headings" do
-      output = processor.render markdown_table
       expect(output).to include('C')
     end
 
     it "treats cells starting with # with more complex markup as row headings" do
-      output = processor.render markdown_table
       expect(output).to match(/G<\/em>\s*<\/th>/)
     end
 
     it "treats other cells as ordinary cells" do
-      output = processor.render markdown_table
       expect(output).to include("D")
       expect(output).to include("E")
       expect(output).to include("F")
       expect(output).to include("H")
     end
   end
-end
-
-RSpec.describe "code blocks" do
-  let(:app) { double("app") }
-  let(:context) { double("context") }
-
-  before :each do
-    allow(context).to receive(:app) { app }
-    allow(app).to receive(:api)
-  end
 
-  describe "without syntax highlighting" do
-    let(:processor) {
-      Redcarpet::Markdown.new(GovukTechDocs::TechDocsHTMLRenderer.new(context: context), fenced_code_blocks: true)
+  describe "#render a code block" do
+    let(:output) {
+      processor.render <<~MARKDOWN
+        Hello world:
+
+        ```ruby
+        def hello_world
+          puts "hello world"
+        end
+        ```
+      MARKDOWN
     }
 
-    describe "#render" do
-      it "sets tab index to 0" do
-        output = processor.render <<~MARKDOWN
-          Hello world:
-
-          ```ruby
-          def hello_world
-            puts "hello world"
-          end
-          ```
-        MARKDOWN
-
-        expect(output).to include('
')
-        expect(output).to include("def hello_world")
-        expect(output).to include('puts "hello world"')
-        expect(output).to include("end")
-      end
+    it "sets tab index to 0" do
+      expect(output).to include('
')
     end
-  end
 
-  describe "with syntax highlighting" do
-    let(:processor) {
-      renderer_class = GovukTechDocs::TechDocsHTMLRenderer.clone.tap do |c|
-        c.send :include, Middleman::Syntax::RedcarpetCodeRenderer
-      end
-
-      Redcarpet::Markdown.new(renderer_class.new(context: context), fenced_code_blocks: true)
-    }
-
-    describe "#render" do
-      it "sets tab index to 0" do
-        output = processor.render <<~MARKDOWN
-          Hello world:
-
-          ```ruby
-          def hello_world
-            puts "hello world"
-          end
-          ```
-        MARKDOWN
+    it "renders the code without syntax highlighting" do
+      expect(output).to include("def hello_world")
+      expect(output).to include('puts "hello world"')
+      expect(output).to include("end")
+    end
 
-        expect(output).to include('
')
-        expect(output).to include("def")
-        expect(output).to include("hello_world")
-        expect(output).to include("puts")
-        expect(output).to include('"hello world"')
-        expect(output).to include("end")
+    describe "with syntax highlighting" do
+      let(:processor) {
+        renderer_class = described_class.clone.tap do |c|
+          c.send :include, Middleman::Syntax::RedcarpetCodeRenderer
+        end
+        Redcarpet::Markdown.new(renderer_class.new(context: context), tables: true, fenced_code_blocks: true)
+      }
+
+      describe "#render a code block" do
+        it "sets tab index to 0" do
+          expect(output).to include('
')
+        end
+
+        it "renders the code with syntax highlighting" do
+          expect(output).to include("def")
+          expect(output).to include("hello_world")
+          expect(output).to include("puts")
+          expect(output).to include('"hello world"')
+          expect(output).to include("end")
+        end
       end
     end
   end

From e11be57cce13fd4559a31bdfae90c49f83795519 Mon Sep 17 00:00:00 2001
From: Richard Towers 
Date: Fri, 12 Mar 2021 13:43:56 +0000
Subject: [PATCH 4/6] Replace regex with nokogiri to process code blocks

---
 lib/govuk_tech_docs/tech_docs_html_renderer.rb | 18 +++++++++++++++---
 .../tech_docs_html_renderer_spec.rb            |  2 +-
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/lib/govuk_tech_docs/tech_docs_html_renderer.rb b/lib/govuk_tech_docs/tech_docs_html_renderer.rb
index 86416a14..cda21691 100644
--- a/lib/govuk_tech_docs/tech_docs_html_renderer.rb
+++ b/lib/govuk_tech_docs/tech_docs_html_renderer.rb
@@ -83,13 +83,25 @@ def block_code(text, lang)
         # to include Middleman::Syntax::RedcarpetCodeRenderer. This defines its own
         # version of `block_code(text, lang)` which we can call with `super`.
 
-        highlighted_html = super
-        highlighted_html.sub("
#{text}
"
+        fragment = Nokogiri::HTML::DocumentFragment.parse("")
+        pre = Nokogiri::XML::Node.new "pre", fragment
+        pre["tabindex"] = "0"
+        code = Nokogiri::XML::Node.new "code", fragment
+        code["class"] = lang
+        code.content = text
+        pre.add_child code
+        pre.to_html
       end
     end
   end
diff --git a/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb b/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb
index 05efa2f2..85b3dc5d 100644
--- a/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb
+++ b/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb
@@ -77,7 +77,7 @@ def hello_world
 
       describe "#render a code block" do
         it "sets tab index to 0" do
-          expect(output).to include('
')
+          expect(output).to include('
')
         end
 
         it "renders the code with syntax highlighting" do

From 43e362038cc194226df8ea1d74034134b888f03c Mon Sep 17 00:00:00 2001
From: Richard Towers 
Date: Fri, 12 Mar 2021 13:54:57 +0000
Subject: [PATCH 5/6] Update CHANGELOG to add tabbable code blocks

---
 CHANGELOG.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ca808404..5438f960 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,7 @@
 - [#210: Fix issue with WCAG 2.1 success criterion 1.3.1 (Info and Relationships)](https://github.com/alphagov/tech-docs-gem/pull/210)
 - [#209: Some search and keyboard navigation updates](https://github.com/alphagov/tech-docs-gem/pull/209)
 - [#214: Implement row level table headings to allow accessible tables with row headings](https://github.com/alphagov/tech-docs-gem/pull/214)
+- [#215: Add tabindex and focus states to code blocks](https://github.com/alphagov/tech-docs-gem/pull/215)
 
 ### Ruby version bump
 

From a0c9bfddbc2749514a17c867ac3f3512a388542d Mon Sep 17 00:00:00 2001
From: Richard Towers 
Date: Fri, 12 Mar 2021 14:28:35 +0000
Subject: [PATCH 6/6] Review: tidy up structure of tests

- one describe block to describe what you're testing
- context blocks to specify the conditions of the tests
---
 .../tech_docs_html_renderer_spec.rb           | 45 ++++++++++---------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb b/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb
index 85b3dc5d..83723aad 100644
--- a/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb
+++ b/spec/govuk_tech_docs/tech_docs_html_renderer_spec.rb
@@ -57,36 +57,41 @@ def hello_world
       MARKDOWN
     }
 
-    it "sets tab index to 0" do
-      expect(output).to include('
')
-    end
+    context "without syntax highlighting" do
+      let(:processor) {
+        Redcarpet::Markdown.new(described_class.new(context: context), fenced_code_blocks: true)
+      }
+
+      it "sets tab index to 0" do
+        expect(output).to include('
')
+      end
 
-    it "renders the code without syntax highlighting" do
-      expect(output).to include("def hello_world")
-      expect(output).to include('puts "hello world"')
-      expect(output).to include("end")
+      it "renders the code without syntax highlighting" do
+        expect(output).to include("def hello_world")
+        expect(output).to include('puts "hello world"')
+        expect(output).to include("end")
+      end
     end
 
-    describe "with syntax highlighting" do
+
+    context "with syntax highlighting" do
       let(:processor) {
         renderer_class = described_class.clone.tap do |c|
           c.send :include, Middleman::Syntax::RedcarpetCodeRenderer
         end
-        Redcarpet::Markdown.new(renderer_class.new(context: context), tables: true, fenced_code_blocks: true)
+        Redcarpet::Markdown.new(renderer_class.new(context: context), fenced_code_blocks: true)
       }
 
-      describe "#render a code block" do
-        it "sets tab index to 0" do
-          expect(output).to include('
')
-        end
+      it "sets tab index to 0" do
+        expect(output).to include('
')
+      end
 
-        it "renders the code with syntax highlighting" do
-          expect(output).to include("def")
-          expect(output).to include("hello_world")
-          expect(output).to include("puts")
-          expect(output).to include('"hello world"')
-          expect(output).to include("end")
-        end
+      it "renders the code with syntax highlighting" do
+        expect(output).to include("def")
+        expect(output).to include("hello_world")
+        expect(output).to include("puts")
+        expect(output).to include('"hello world"')
+        expect(output).to include("end")
       end
     end
   end