From e78fe663d20ca5b0add13e84548ef0fffec162b8 Mon Sep 17 00:00:00 2001 From: binarygit Date: Sat, 4 Nov 2023 17:01:02 +0545 Subject: [PATCH 1/6] Open external links in product description in a new page --- app/assets/javascripts/templates/product_modal.html.haml | 2 +- app/webpacker/controllers/richtext_controller.js | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 app/webpacker/controllers/richtext_controller.js diff --git a/app/assets/javascripts/templates/product_modal.html.haml b/app/assets/javascripts/templates/product_modal.html.haml index 91021838724..ec9cb00625e 100644 --- a/app/assets/javascripts/templates/product_modal.html.haml +++ b/app/assets/javascripts/templates/product_modal.html.haml @@ -11,7 +11,7 @@ %filter-selector{ 'selector-set' => "productPropertySelectors", objects: "[product] | propertiesWithValuesOf" } .product-description{"ng-if" => "product.description_html"} - %p.text-small{"ng-bind-html" => "::product.description_html"} + %p.text-small{"ng-bind-html" => "::product.description_html", "data-controller" => "richtext"} .columns.small-12.medium-6.large-6.product-img %img{"ng-src" => "{{::product.largeImage}}", "ng-if" => "::product.largeImage"} diff --git a/app/webpacker/controllers/richtext_controller.js b/app/webpacker/controllers/richtext_controller.js new file mode 100644 index 00000000000..ad54dd929aa --- /dev/null +++ b/app/webpacker/controllers/richtext_controller.js @@ -0,0 +1,9 @@ +import { Controller } from "stimulus"; + +export default class extends Controller { + connect() { + this.element.querySelectorAll("a").forEach(function (link) { + link.target = "_blank"; + }); + } +} From f52c7e8a5dd149ac4d99020564ebae33e3ead6d2 Mon Sep 17 00:00:00 2001 From: binarygit Date: Thu, 9 Nov 2023 21:30:03 +0545 Subject: [PATCH 2/6] Rename richtext controller and write specs --- .../templates/product_modal.html.haml | 2 +- ...ler.js => add_blank_to_link_controller.js} | 0 .../add_blank_to_link_controller_test.js | 24 +++++++++++++++++++ .../system/consumer/shopping/products_spec.rb | 7 +++--- 4 files changed, 29 insertions(+), 4 deletions(-) rename app/webpacker/controllers/{richtext_controller.js => add_blank_to_link_controller.js} (100%) create mode 100644 spec/javascripts/stimulus/add_blank_to_link_controller_test.js diff --git a/app/assets/javascripts/templates/product_modal.html.haml b/app/assets/javascripts/templates/product_modal.html.haml index ec9cb00625e..374f1bda4ed 100644 --- a/app/assets/javascripts/templates/product_modal.html.haml +++ b/app/assets/javascripts/templates/product_modal.html.haml @@ -11,7 +11,7 @@ %filter-selector{ 'selector-set' => "productPropertySelectors", objects: "[product] | propertiesWithValuesOf" } .product-description{"ng-if" => "product.description_html"} - %p.text-small{"ng-bind-html" => "::product.description_html", "data-controller" => "richtext"} + %p.text-small{"ng-bind-html" => "::product.description_html", "data-controller" => "add-blank-to-link"} .columns.small-12.medium-6.large-6.product-img %img{"ng-src" => "{{::product.largeImage}}", "ng-if" => "::product.largeImage"} diff --git a/app/webpacker/controllers/richtext_controller.js b/app/webpacker/controllers/add_blank_to_link_controller.js similarity index 100% rename from app/webpacker/controllers/richtext_controller.js rename to app/webpacker/controllers/add_blank_to_link_controller.js diff --git a/spec/javascripts/stimulus/add_blank_to_link_controller_test.js b/spec/javascripts/stimulus/add_blank_to_link_controller_test.js new file mode 100644 index 00000000000..97fdc884e0a --- /dev/null +++ b/spec/javascripts/stimulus/add_blank_to_link_controller_test.js @@ -0,0 +1,24 @@ +/** + * @jest-environment jsdom + */ + +import { Application } from "stimulus"; +import add_blank_to_link_controller from "../../../app/webpacker/controllers/add_blank_to_link_controller"; + +describe("AddBlankToLink", () => { + beforeAll(() => { + const application = Application.start(); + application.register("add-blank-to-link", add_blank_to_link_controller); + }); + + describe("#add-blank-to-link", () => { + beforeEach(() => { + document.body.innerHTML = `
www.ofn.com
`; + }); + + it("adds target='_blank' to anchor tags", () => { + const anchorTag = document.querySelector('a') + expect(anchorTag.getAttribute('target')).toBe("_blank"); + }); + }); +}); diff --git a/spec/system/consumer/shopping/products_spec.rb b/spec/system/consumer/shopping/products_spec.rb index d568095ec56..6e14e5ca78f 100644 --- a/spec/system/consumer/shopping/products_spec.rb +++ b/spec/system/consumer/shopping/products_spec.rb @@ -76,7 +76,7 @@ product.description = '

Formatted product description: Lorem ipsum dolor sit amet, consectetur adipiscing elit. Morbi venenatis metus diam, eget scelerisque nibh auctor non.

Link to an ' \ - 'external site' \ + 'external site' \ 'open food network logo' product.save! @@ -84,7 +84,7 @@ visit shop_path expect(page).to have_content product.name expect_product_description_html_to_be_displayed(product, product.description, nil, - truncate: true) + truncate: true, href: true) end it "does not show unsecure HTML" do @@ -144,7 +144,7 @@ end def expect_product_description_html_to_be_displayed(product, html, not_include = nil, - truncate: false) + truncate: false, href: false) # check inside list of products within "#product-#{product.id} .product-description" do expect(html).to include(html) @@ -157,6 +157,7 @@ def expect_product_description_html_to_be_displayed(product, html, not_include = expect(page).to have_selector '.reveal-modal' modal_should_be_open_for product within(".reveal-modal") do + expect(find_link('external site')[:target]).to eq('_blank') if href expect(html).to include(html) expect(html).not_to include(not_include) if not_include end From 249c0029dd590a3908ee459cc831c3cbb8ae9a2c Mon Sep 17 00:00:00 2001 From: binarygit Date: Mon, 13 Nov 2023 13:53:05 +0545 Subject: [PATCH 3/6] Improve tests --- spec/system/consumer/shopping/products_spec.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/system/consumer/shopping/products_spec.rb b/spec/system/consumer/shopping/products_spec.rb index 6e14e5ca78f..dd3dd08066f 100644 --- a/spec/system/consumer/shopping/products_spec.rb +++ b/spec/system/consumer/shopping/products_spec.rb @@ -147,14 +147,12 @@ def expect_product_description_html_to_be_displayed(product, html, not_include = truncate: false, href: false) # check inside list of products within "#product-#{product.id} .product-description" do - expect(html).to include(html) expect(html).not_to include(not_include) if not_include expect(page).to have_content "..." if truncate # it truncates a long product description end # check in product description modal click_link product.name - expect(page).to have_selector '.reveal-modal' modal_should_be_open_for product within(".reveal-modal") do expect(find_link('external site')[:target]).to eq('_blank') if href From d0c07df0cfe5d79d7b0b691d6c0de1c5b4e90991 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 15 Nov 2023 10:06:48 +1100 Subject: [PATCH 4/6] Avoid complexity of branch flags --- .../system/consumer/shopping/products_spec.rb | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/spec/system/consumer/shopping/products_spec.rb b/spec/system/consumer/shopping/products_spec.rb index dd3dd08066f..4d5f41ab781 100644 --- a/spec/system/consumer/shopping/products_spec.rb +++ b/spec/system/consumer/shopping/products_spec.rb @@ -83,8 +83,14 @@ visit shop_path expect(page).to have_content product.name + + # It truncates a long product description. + within_product_description(product) do + expect(html).to include product.description + expect(page).to have_content "..." + end expect_product_description_html_to_be_displayed(product, product.description, nil, - truncate: true, href: true) + href: true) end it "does not show unsecure HTML" do @@ -94,8 +100,12 @@ visit shop_path expect(page).to have_content product.name + within_product_description(product) do + expect(html).to include "

Safe

" + expect(html).not_to include "" + end expect_product_description_html_to_be_displayed( - product, "

Safe

", "", truncate: false + product, "

Safe

", "" ) end end @@ -144,13 +154,7 @@ end def expect_product_description_html_to_be_displayed(product, html, not_include = nil, - truncate: false, href: false) - # check inside list of products - within "#product-#{product.id} .product-description" do - expect(html).not_to include(not_include) if not_include - expect(page).to have_content "..." if truncate # it truncates a long product description - end - + href: false) # check in product description modal click_link product.name modal_should_be_open_for product @@ -160,4 +164,8 @@ def expect_product_description_html_to_be_displayed(product, html, not_include = expect(html).not_to include(not_include) if not_include end end + + def within_product_description(product, &) + within("#product-#{product.id} .product-description", &) + end end From b626ec1cd734270307f2eb14cb86cdecb374c922 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 15 Nov 2023 10:13:24 +1100 Subject: [PATCH 5/6] Further reduce branching This is much easier to read. --- .../system/consumer/shopping/products_spec.rb | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/spec/system/consumer/shopping/products_spec.rb b/spec/system/consumer/shopping/products_spec.rb index 4d5f41ab781..ef4fef65f36 100644 --- a/spec/system/consumer/shopping/products_spec.rb +++ b/spec/system/consumer/shopping/products_spec.rb @@ -89,8 +89,10 @@ expect(html).to include product.description expect(page).to have_content "..." end - expect_product_description_html_to_be_displayed(product, product.description, nil, - href: true) + within_product_modal(product) do + expect(html).to include product.description + expect(find_link('external site')[:target]).to eq('_blank') + end end it "does not show unsecure HTML" do @@ -104,9 +106,10 @@ expect(html).to include "

Safe

" expect(html).not_to include "" end - expect_product_description_html_to_be_displayed( - product, "

Safe

", "" - ) + within_product_modal(product) do + expect(html).to include "

Safe

" + expect(html).not_to include "" + end end end @@ -153,16 +156,10 @@ end end - def expect_product_description_html_to_be_displayed(product, html, not_include = nil, - href: false) - # check in product description modal + def within_product_modal(product, &) click_link product.name modal_should_be_open_for product - within(".reveal-modal") do - expect(find_link('external site')[:target]).to eq('_blank') if href - expect(html).to include(html) - expect(html).not_to include(not_include) if not_include - end + within(".reveal-modal", &) end def within_product_description(product, &) From 58d2e9d9aa5eb1612ef90b3b65e615adc57f97cd Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 15 Nov 2023 10:28:45 +1100 Subject: [PATCH 6/6] Fix pending spec example and clarify displayed content --- spec/system/consumer/shopping/products_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/system/consumer/shopping/products_spec.rb b/spec/system/consumer/shopping/products_spec.rb index ef4fef65f36..c7cec7efa22 100644 --- a/spec/system/consumer/shopping/products_spec.rb +++ b/spec/system/consumer/shopping/products_spec.rb @@ -86,7 +86,7 @@ # It truncates a long product description. within_product_description(product) do - expect(html).to include product.description + expect(html).to include "Formatted product description: Lorem ipsum" expect(page).to have_content "..." end within_product_modal(product) do @@ -105,10 +105,12 @@ within_product_description(product) do expect(html).to include "

Safe

" expect(html).not_to include "" + expect(page).to have_content "alert('Dangerous!'); Safe" end within_product_modal(product) do expect(html).to include "

Safe

" expect(html).not_to include "" + expect(page).to have_content "alert('Dangerous!'); Safe" end end end