-
-
Notifications
You must be signed in to change notification settings - Fork 724
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open external links in product description in a new page #11761
Changes from all commits
e78fe66
f52c7e8
249c002
d0c07df
b626ec1
58d2e9d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import { Controller } from "stimulus"; | ||
|
||
export default class extends Controller { | ||
connect() { | ||
this.element.querySelectorAll("a").forEach(function (link) { | ||
link.target = "_blank"; | ||
}); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 = `<div data-controller="add-blank-to-link"><a href="www.ofn.com">www.ofn.com</a></div>`; | ||
}); | ||
|
||
it("adds target='_blank' to anchor tags", () => { | ||
const anchorTag = document.querySelector('a') | ||
expect(anchorTag.getAttribute('target')).toBe("_blank"); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,15 +76,23 @@ | |
product.description = '<p><b>Formatted</b> product description: Lorem ipsum dolor sit amet, | ||
consectetur adipiscing elit. Morbi venenatis metus diam, | ||
eget scelerisque nibh auctor non. </p> Link to an ' \ | ||
'<a href="http://google.fr" target="_blank">external site</a>' \ | ||
'<a href="http://google.fr">external site</a>' \ | ||
'<img src="https://www.openfoodnetwork.org/wp-content/uploads/2019/' \ | ||
'05/[email protected]" alt="open food network logo" />' | ||
product.save! | ||
|
||
visit shop_path | ||
expect(page).to have_content product.name | ||
expect_product_description_html_to_be_displayed(product, product.description, nil, | ||
truncate: true) | ||
|
||
# It truncates a long product description. | ||
within_product_description(product) do | ||
expect(html).to include "<b>Formatted</b> product description: Lorem ipsum" | ||
expect(page).to have_content "..." | ||
end | ||
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 | ||
|
@@ -94,9 +102,16 @@ | |
visit shop_path | ||
expect(page).to have_content product.name | ||
|
||
expect_product_description_html_to_be_displayed( | ||
product, "<p>Safe</p>", "<script>alert('Dangerous!');</script>", truncate: false | ||
) | ||
within_product_description(product) do | ||
expect(html).to include "<p>Safe</p>" | ||
expect(html).not_to include "<script>alert('Dangerous!');</script>" | ||
expect(page).to have_content "alert('Dangerous!'); Safe" | ||
end | ||
within_product_modal(product) do | ||
expect(html).to include "<p>Safe</p>" | ||
expect(html).not_to include "<script>alert('Dangerous!');</script>" | ||
expect(page).to have_content "alert('Dangerous!'); Safe" | ||
end | ||
end | ||
end | ||
|
||
|
@@ -143,22 +158,13 @@ | |
end | ||
end | ||
|
||
def expect_product_description_html_to_be_displayed(product, html, not_include = nil, | ||
truncate: false) | ||
# check inside list of products | ||
within "#product-#{product.id} .product-description" do | ||
expect(html).to include(html) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I now realise that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice. Thanks @mkllnk. This was a good learning experience 🙏 |
||
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 | ||
def within_product_modal(product, &) | ||
click_link product.name | ||
expect(page).to have_selector '.reveal-modal' | ||
modal_should_be_open_for product | ||
within(".reveal-modal") do | ||
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, &) | ||
within("#product-#{product.id} .product-description", &) | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what the user sees.