Skip to content

Commit

Permalink
Fix inconsistent behaviour between image syntaxes
Browse files Browse the repository at this point in the history
There was inconsistent behaviour between the different syntaxes used for
embedding images.

The `!!n` syntax must start on a new line. But the previous line can
contain other content (e.g. paragraph text).

For example, this is valid:

```
Some example content, immediately followed by an image.
!!1
```

Whereas the `[Image:]` syntax had to be preceded by two newline
characters (i.e. a new Markdown paragraph).

For example:

```
This does not work:
[Image: example.jpg]

But this does because it's preceded by two newline characters:

[Image: example.jpg]
```

This inconsistent behaviour was spotted by publishers after we
implemented support for the `[Image:]` syntax in Whitehall Publisher,
since they're used to the `!!n` syntax which is more lenient.

We've decided to bring the two in line by adjusting the `[Image:]`
syntax so that it only needs to be preceded by one newline. This still
avoids the issue identified in 663e099
because it does not allow images to be embedded within a paragraph.

I've tweaked the `[Image:]` regex and adding extra tests to cover the
desired behaviour. I've also added equivalent coverage to the `!!n`
syntax tests to explicitly call out this behaviour and ensure the two
syntaxes behave consistently.
  • Loading branch information
ollietreend committed May 18, 2023
1 parent 24cef39 commit 7dc11bb
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/govspeak.rb
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ def self.devolved_options
renderer.render(contact: ContactPresenter.new(contact))
end

extension("Image", /#{NEW_PARAGRAPH_LOOKBEHIND}\[Image:\s*(.*?)\s*\]/) do |image_id|
extension("Image", /^\[Image:\s*(.*?)\s*\]/) do |image_id|
image = images.detect { |c| c.is_a?(Hash) && c[:id] == image_id }
next "" unless image

Expand Down
26 changes: 26 additions & 0 deletions test/govspeak_images_bang_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,30 @@ def initialize(attrs = {})
)
end
end

test "!!n syntax must start on a new line" do
given_govspeak "some text !!1", images: [Image.new] do
assert_html_output("<p>some text !!1</p>")
end

given_govspeak "!!1", images: [Image.new] do
assert_html_output(
"<figure class=\"image embedded\"><div class=\"img\"><img src=\"http://example.com/image.jpg\" alt=\"my alt\"></div></figure>",
)
end

given_govspeak "!!1 some text", images: [Image.new] do
assert_html_output(
"<figure class=\"image embedded\"><div class=\"img\"><img src=\"http://example.com/image.jpg\" alt=\"my alt\"></div></figure>\n<p>some text</p>",
)
end

given_govspeak "some text\n!!1\nsome more text", images: [Image.new] do
assert_html_output <<~HTML
<p>some text</p>
<figure class="image embedded"><div class="img"><img src="http://example.com/image.jpg" alt="my alt"></div></figure>
<p>some more text</p>
HTML
end
end
end
8 changes: 8 additions & 0 deletions test/govspeak_images_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,5 +88,13 @@ def build_image(attrs = {})
"<figure class=\"image embedded\"><div class=\"img\"><img src=\"http://example.com/image.jpg\" alt=\"my alt\"></div></figure>\n<p>some text</p>",
)
end

given_govspeak "some text\n[Image:image-id]\nsome more text", images: [build_image] do
assert_html_output <<~HTML
<p>some text</p>
<figure class="image embedded"><div class="img"><img src="http://example.com/image.jpg" alt="my alt"></div></figure>
<p>some more text</p>
HTML
end
end
end

0 comments on commit 7dc11bb

Please sign in to comment.