Skip to content
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

Support links opening in new tab #363

Merged
merged 6 commits into from
Dec 26, 2022
Merged

Conversation

peteryates
Copy link
Member

@peteryates peteryates commented Aug 21, 2022

This PR adds new keyword argument new_tab to #govuk_link_to and #govuk_button_link_to that makes it easier (and safer) for developers to create links that open in a new tab/window.

The design system specifies that the attributes rel="noreferrer noopener" target="_blank" should be present to prevent reverse tabnabbing. This is often forgotten by developers.

The default value for new_tab is false. The new behaviours are as follows:

  • when new_tab: false - no change, the helper works in the normal fashion
  • when new_tab: true - the default suffix is added after the link text
  • when new_tab: "a string" - the supplied suffix is added after the link text

The default suffix is "(opens in new tab)".

Fixes #360

@netlify
Copy link

netlify bot commented Aug 21, 2022

Deploy Preview for govuk-components ready!

Name Link
🔨 Latest commit 9570c4c
🔍 Latest deploy log https://app.netlify.com/sites/govuk-components/deploys/63a9b2aa7cc5d70008f53727
😎 Deploy Preview https://deploy-preview-363--govuk-components.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@peteryates peteryates force-pushed the links-that-open-in-a-new-tab branch 2 times, most recently from 065eae3 to dfc0893 Compare August 22, 2022 21:48
@peteryates peteryates force-pushed the links-that-open-in-a-new-tab branch from dfc0893 to de317d3 Compare September 5, 2022 08:38
@frankieroberto
Copy link
Collaborator

Looks good! Might be worth checking if this plays nicely if you were to also specify those rel and target attributes as options (even though this seems unlikely)?

eg:

govuk_link_to "Blah", "https://www.example.com", {rel: "help", target: "new"}, new_tab: "true"

Behaviour here is a bit unpredictable, but possibly the ideal would be to:

  • append noreferrer noopener to the rel value given, separated by a space
  • the target value given manually should override the _blank one (or possibly the other way round, hard to know...)

@peteryates
Copy link
Member Author

peteryates commented Sep 6, 2022

Behaviour here is a bit unpredictable, but possibly the ideal would be to:

  • append noreferrer noopener to the rel value given, separated by a space
  • the target value given manually should override the _blank one (or possibly the other way round, hard to know...)

This should be quite straightfowrard since this change to html-attributes-utils.

Unfortunately as we can see from the builds, Ruby 3.0.0 changed the way keyword arguments meaning that the approach here needs to be adjusted.

I'm not sure it's possible to change without breaking Rails compatibility or nesting the params inside the options hash.

@peteryates
Copy link
Member Author

Going to hold off on this until Ruby 3.2.0 is released at Christmas and support for Ruby 2.7 is dropped.

@peteryates
Copy link
Member Author

This PR can follow on from #381 and will be in the upcoming release 🥳

The new tab argument injects some extra HTML attributes that are
suggested by the GOV.UK Design System.
When new tab is:

* false    - nothing happens
* true     - the default text appended to the link
* a string - the supplied text is appended to the link
Now we've added a keyword argument to the link helper we need to tell
Ruby that the third arg is a hash and not a keyword argument.

This is likely to be a breaking change.
@peteryates peteryates force-pushed the links-that-open-in-a-new-tab branch from de8dcaf to b7563f9 Compare December 26, 2022 12:57
We're going to use deep merge on the options passed to govuk_link_helper
and version 1.0.0 adds 'rel' to the list of mergeable items
This will allow us to merge the default and provided params in a way
that overwrites the target attribute but combines the rel

Add a test that covers the behaviour based on @frankieroberto's
suggestion in the thread.

#363
@peteryates peteryates marked this pull request as ready for review December 26, 2022 13:50
@peteryates
Copy link
Member Author

peteryates commented Dec 26, 2022

Behaviour here is a bit unpredictable, but possibly the ideal would be to:

  • append noreferrer noopener to the rel value given, separated by a space
  • the target value given manually should override the _blank one (or possibly the other way round, hard to know...)

This behaviour is now included and covered by these specs:

https://github.com/DFE-Digital/govuk-components/blob/9570c4cab0c7e450dc87d1d018d7fd2e6ca56642/spec/helpers/govuk_link_helper_spec.rb#L139-L151

@peteryates peteryates merged commit 8336bb8 into main Dec 26, 2022
@peteryates peteryates deleted the links-that-open-in-a-new-tab branch December 26, 2022 15:03
@peteryates peteryates mentioned this pull request Feb 17, 2023
4 tasks
@peteryates peteryates mentioned this pull request Jun 12, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Idea: make it easier to create links that open in new windows
2 participants