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 Link Icon for Images? #589

Closed
latenitefilms opened this issue Jul 25, 2023 · 10 comments
Closed

External Link Icon for Images? #589

latenitefilms opened this issue Jul 25, 2023 · 10 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@latenitefilms
Copy link

@geoffreymcgill - having an external link icon for images that have a link seems a little weird? For example:

image

I feel like the new outbound feature should only apply to text links? Thoughts?

@geoffreymcgill geoffreymcgill self-assigned this Jul 25, 2023
@geoffreymcgill geoffreymcgill added the bug Something isn't working label Jul 25, 2023
@geoffreymcgill
Copy link
Collaborator

This is a defect. During our testing, somehow we did not notice this case. 🤦‍♂️

We will remove the icon, but would you prefer if the other properties of outbound still apply? such as target and custom? Feels like they should.

This should be an easy fix and we will make a v3.1.1 patch release as soon as it's fixed.

@latenitefilms
Copy link
Author

Yes, I think they should too.

@geoffreymcgill
Copy link
Collaborator

There are a few issues going on here and we are working on some solutions.

As a temporary fix, you can remove the {target="_blank"} custom attribute on that link.

After the next release, that custom attribute will not be required anyways, so removing now is not a problem and it will just start working as expected with the next release.

I will keep this thread updated with our progress.

@latenitefilms
Copy link
Author

We currently use {target="_blank"} a LOT on FCP Cafe. Am I safe to just do a global find and replace to remove all of them?

It might also be worth clarifying if the exclude parameter formatting? For example, is this valid or do I need to add a wildcard, etc?

outbound:
  exclude:
    - github.com/CommandPost/FCPCafe/

@geoffreymcgill
Copy link
Collaborator

If the path of the link is to an external URL, Retype will treat as outbound links if you remove the {target="_blank"} custom attribute. An external URL would be one that starts with https or http.

If the path is internal and you remove the {target="_blank"} custom attribute, then Retype will not treat the link as an external outbound link and clicking the link will not open in a new tab.

If noticed the following scenario in your project:

[Make a suggestion!](/contribute/){target="_blank"}

If {target="_blank"} is removed, the link will not open in a new tab, as Retype would classify as an internal link.

is this valid or do I need to add a wildcard, etc?

Yes, it is valid. Your exclude will exclude all outbound links that contain the value github.com/CommandPost/FCPCafe/ anywhere in the path.

outbound:
  exclude:
    - github.com/CommandPost/FCPCafe/

Hope this helps.

@latenitefilms
Copy link
Author

If noticed the following scenario in your project:

Good spotting! I'm not sure if I did that deliberately or not, but seems like a mistake regardless. Thanks!

@geoffreymcgill
Copy link
Collaborator

geoffreymcgill commented Jul 26, 2023

I also noticed the following link:

[here](https://github.com/CommandPost/FCPCafe/graphs/contributors){target="_blank"}

but you exclude github.com/CommandPost/FCPCafe/ from the outbound functionality.

No big deal here, but if you remove {target="_blank"} from that link, it will not open in a new Tab as Retype excludes that link from the outbound parsing.


You have a lot of links like [GitHub](https://github.com){target="_blank"} or [Retype](https://retype.com){target="_blank"}, and removing the {target="_blank"} would help clean up your content. There would be no change after removing {target="_blank"} because Retype already treats those links as outbound and sets the target="_blank".


Then I also noticed [Markdown](https://www.markdownguide.org) on one of your pages, without {target="_blank"}. The nice thing about the new outbound functionality is that Retype automatically found that external/outbound link and will open in a new Tab, even though you might have mistakingly missed adding the {target="_blank"} custom attribute.


All those are minor issues, but removing as many (or all of) the {target="_blank"} custom attributes as you can will help clean up your content. It's better to rely on the parser to find all the external links and then just tweak things with the exclude and include configs.

Hope this helps.

@geoffreymcgill
Copy link
Collaborator

geoffreymcgill commented Jul 26, 2023

Using the regex search pattern (\[.*\]\(http.*\))\{target="_blank"\} should find all the paths with outbound links. Looks like there are at least 730 instances, maybe more.

Screen Shot 2023-07-25 at 6 53 48 PM

Once those outbound links are cleaned up, then you can just search for {target="_blank"} to find any remaining instances that should only be set on internal links. Probably just remove all those instances.

Plus I see you have some {target="_blank"} instances in your .js scripts. Good idea to clean those up too.

@geoffreymcgill geoffreymcgill modified the milestone: v3.1 Jul 26, 2023
@geoffreymcgill
Copy link
Collaborator

This issue has been fixed and the fix will be included in the upcoming Retype v3.2 release, which should happen very soon.

@latenitefilms Thanks for reporting this issue!

@geoffreymcgill
Copy link
Collaborator

Retype v3.2 has been released.

@geoffreymcgill geoffreymcgill added this to the v3.2 milestone Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants