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

external links in rich text blocks now opening in new tab #8195

Merged
merged 7 commits into from
Feb 9, 2022

Conversation

danielfmiranda
Copy link
Collaborator

@danielfmiranda danielfmiranda commented Feb 4, 2022

Closes #8002

Link to sample test page: https://foundation-s-rich-text--1wiazw.mofostaging.net/en/blog/initial-test-blog-post-with-fixed-title/

Steps to test:

  1. Please visit the test page above
  2. In the posts content, please click on any of the external links such as the ones that say "This is a link to a fake URL!" or the ones found in the image or quote block.
  3. These all should open to a new tab
  4. If you would like to test another page type, visit https://foundation-s-rich-text--1wiazw.mofostaging.net/en/publication-page-with-child-article-pages/fixed-title-article-page/ to test out the external rich text links found there.
  5. If everything is working as expected, testing is complete!

@mofodevops mofodevops temporarily deployed to foundation-s-rich-text--1wiazw February 4, 2022 21:15 Inactive
@danielfmiranda danielfmiranda temporarily deployed to foundation-s-rich-text--1wiazw February 4, 2022 21:54 Inactive
@danielfmiranda danielfmiranda temporarily deployed to foundation-s-rich-text--1wiazw February 4, 2022 23:37 Inactive
@danielfmiranda danielfmiranda temporarily deployed to foundation-s-rich-text--1wiazw February 7, 2022 19:39 Inactive
@@ -84,6 +86,21 @@ def register_large_feature(features):
features.default_features.append('large')


# Updating external links in rich text blocks to open in a new tab
class RichTextExternalLinkNewTabHandler(LinkHandler):
Copy link
Collaborator Author

@danielfmiranda danielfmiranda Feb 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs Referenced: https://docs.wagtail.org/en/latest/extending/rich_text_internals.html#registering-rewrite-handlers

What this new block is doing is adding a new handler for rich text links, and if they are external, add the target='_blank' to open in a new tab

According to whatwg/html#4078 we no longer need to set noopener nofollower as these are set by default
@danielfmiranda danielfmiranda temporarily deployed to foundation-s-rich-text--1wiazw February 7, 2022 20:03 Inactive
Copy link
Contributor

@Pomax Pomax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codewise this looks good, although it of course only works for richtext links so we may be setting up inconsistent behaviour for our users.

e.g. all links open in a new window, but not this one, because it's not a richtext link:

image

@kristinashu
Copy link

Thank you for setting up the examples Daniel! The link types you listed are all working well. But I did notice the external links in the captions and the external button link are still opening in the same tab. Is that intentional, is it possible to make them open in a new tab as well?

image

@danielfmiranda
Copy link
Collaborator Author

Hi @kristinashu! That was intentional, from my understanding, the ticket asked for only links in rich text fields.
However if you would prefer that all external links or a specific list of link types on the site open in a new tab, that is definitely doable as well!

Should I create a new ticket for that or should I add it to this ticket?

Thank you!

@kristinashu
Copy link

Ah ok thank you for explaining. Yes, perhaps open a new ticket to tackle any remaining external links. Will approve this one now tho!

@danielfmiranda
Copy link
Collaborator Author

Sounds good @kristinashu thanks for the approval!

I have created #8214 to tackle the rest of the links on the page 😃

@danielfmiranda danielfmiranda temporarily deployed to foundation-s-rich-text--1wiazw February 9, 2022 19:15 Inactive
@danielfmiranda danielfmiranda merged commit b466bc4 into main Feb 9, 2022
@danielfmiranda danielfmiranda deleted the rich_text_external_link branch February 9, 2022 19:24
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.

Update rich text streamfield blocks to open external links in a new tab
4 participants