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

Idea: make it easier to create links that open in new windows #360

Closed
peteryates opened this issue Aug 16, 2022 · 4 comments · Fixed by #363
Closed

Idea: make it easier to create links that open in new windows #360

peteryates opened this issue Aug 16, 2022 · 4 comments · Fixed by #363

Comments

@peteryates
Copy link
Member

peteryates commented Aug 16, 2022

According to the design system, links that open in new tabs/windows need to:

  • have target: "_blank"
  • be rel: "noreferrer noopener" (to prevent reverse tabnabbing)
  • include a description that informs the user the window will open in a new window, usually (opens in new tab) or similar

Rather than having to remember all of these things every time, we could bake the functionality into the library. It could be implemented as:

  • a new helper - perhaps #govuk_external_link_to
  • via a keyword argument on #govuk_link_to and friends - maybe external: true

The text appended to the end should be nil by default (leaving it to the dev) but can be defaulted so it always adds the (link opens in new tab) suffix - unless supressed with external_text: nil or similar.

@cpjmcquillan
Copy link
Collaborator

Yeah, this is probably worthwhile.

Teaching Vacancies defined a custom helper for this a while back.

I'd imagine other teams have defined similar helpers too, so I think there is value in extracting something out into this gem.

@peteryates
Copy link
Member Author

That looks perfect, thanks @cpjmcquillan!

Definitely feels like a separate method is the way to go, rather than adding more args to #govuk_link_to.

CTRL + cCTRL + v

@frankieroberto
Copy link
Collaborator

I like this idea, but I think we should avoid using the word external in the helper method name or attributes, as the Design System doesn’t say to open external links in a new tab - most of the time they shouldn't be.

I think the most developer-friendly thing to do would be an additional keyword argument on the existing helper. So perhaps something like this:

<%= govuk_link_to "privacy policy", privacy_policy_path, new_tab: true %>

We could also allow a string to be set instead of a boolean, for cases where you might want to rewrite the additional text shown (eg if translating into another language):

<%= govuk_link_to "privacy policy", privacy_policy_path, new_tab: "(opens in new tab)" %>

(An empty string could be set if you really don’t want to include any additional text)

@peteryates
Copy link
Member Author

peteryates commented Aug 19, 2022

Sounds like a sensible approach @frankieroberto - new_tab feels like the way to go.

  • false by default
  • when true it pulls the text from config (opens in new tab) by default - depends on (Make the library configurable #355)
  • when a string it overrides the default text

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 a pull request may close this issue.

3 participants