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

Add "edit this page" link #157

Merged
merged 1 commit into from
Aug 26, 2024
Merged

Add "edit this page" link #157

merged 1 commit into from
Aug 26, 2024

Conversation

noxilixon
Copy link
Contributor

@noxilixon noxilixon commented Aug 19, 2024

This resolves #132

@noxilixon noxilixon changed the title feat(footer): add edit this page link Add §edit this page§ link Aug 19, 2024
@noxilixon noxilixon changed the title Add §edit this page§ link Add "edit this page" link Aug 19, 2024
Copy link

This is a link to a preview of the website from this Pull Request: https://freifunk.dev/edit-git/.

@noxilixon noxilixon requested a review from Noki August 20, 2024 12:30
Comment on lines 10 to 12
<a class="px-2" href="{{ $Site.Params.editURL }}{{ replace .Dir "\\" "/" }}{{ .LogicalName }}" target="blank">
{{"Edit this page"}}
</a>
Copy link
Member

Choose a reason for hiding this comment

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

I like the functionality, but we should not make this a <a href>-link that could be discovered and followed by search engines. I propose that we mask this link e.g. by using a span with an onclick event and a data attribute:

<span class="px-2" 
      data-url="{{ $Site.Params.editURL }}{{ replace .Dir "\\" "/" }}{{ .LogicalName }}" 
      onclick="window.open(this.getAttribute('data-url'), '_blank');">
    {{"Edit this page"}}
</span>

Copy link
Contributor

Choose a reason for hiding this comment

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

the onlick is dirts and might not work with opening in a new tab or with javascript deactivated.

Wouldn't a rel=”nofollow” suffice?

Copy link
Member

Choose a reason for hiding this comment

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

the onlick is dirts and might not work with opening in a new tab or with javascript deactivated.

Without JavaScript the Freifunk website does not fully work either (e.g. language switcher) and editing at GitHub also requires JavaScript. Without it the content and edit form is not even visible.

Wouldn't a rel=”nofollow” suffice?

The idea behind masking the link is to reduce the number of outgoing links and prevent PageRank from beeing assigned to those links so the remaining internal links will be stronger. nofollow does not do that. With nofollow there will still be PageRank assigned to the links but the PageRank will not be passed on to the target pages. Compared to masking the links nofollow would therefore weaken the site.

We can however just use JavaScript to create the link so it will only become visible when it would also be usable:

<span id="edit-page-link" class="px-2" 
      data-url="{{ $Site.Params.editURL }}{{ replace .Dir "\\" "/" }}{{ .LogicalName }}">
</span>
<script>
document.addEventListener("DOMContentLoaded", function() {
    var editLink = document.getElementById('edit-page-link');
    if (editLink) {
        var url = editLink.getAttribute('data-url');
        editLink.innerText = "Edit this page";
        editLink.onclick = function() {
            window.open(url, '_blank');
        };
    }
});
</script>

@Noki Noki merged commit 5e9ab2b into main Aug 26, 2024
4 checks passed
@Noki Noki deleted the edit-git branch August 26, 2024 18:53
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.

Edit this page link
3 participants