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 link to forked repo below the original #2141

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions source/content.ts
Expand Up @@ -105,6 +105,7 @@ import './features/release-download-count';
import './features/open-issue-to-latest-comment';
import './features/highest-rated-comment';
import './features/clean-issue-filters';
import './features/forked-to';

import './features/scrollable-code-and-blockquote.css';
import './features/center-reactions-popup.css';
Expand Down
154 changes: 154 additions & 0 deletions source/features/forked-to.tsx
@@ -0,0 +1,154 @@
import './more-dropdown.css';
jerone marked this conversation as resolved.
Show resolved Hide resolved
import React from 'dom-chef';
import select from 'select-dom';
import features from '../libs/features';
import cache from '../libs/cache';
import {getRepoURL, getUsername} from '../libs/utils';
import {isOwnRepo} from '../libs/page-detect';

const currentRepo = getRepoURL();
jerone marked this conversation as resolved.
Show resolved Hide resolved

async function init(): Promise<void> {
jerone marked this conversation as resolved.
Show resolved Hide resolved
onForkDialogOpened();

if (isOwnRepo()) {
onForkedPage();
}

await checkForks();
}

// Check for cached forks.
async function checkForks(): Promise<void> {
const repo = getOriginalRepo();
const cached = await getCache(repo);
jerone marked this conversation as resolved.
Show resolved Hide resolved
const validForks = cached.filter(validateFork);
for (const fork of await Promise.all(validForks)) {
if (fork !== currentRepo) {
appendHtml(fork);
storeCache(repo, fork);
jerone marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

// Check if the fork still exists.
async function validateFork(repo: string): Promise<boolean> {
const url = new URL(`/${repo}`, location.href);
try {
const response = await fetch(String(url),
{
method: 'HEAD'
});
return response.ok;
} catch (error) {
return false;
}
jerone marked this conversation as resolved.
Show resolved Hide resolved
}

// Check if we are on a forked page.
function onForkedPage(): void {
jerone marked this conversation as resolved.
Show resolved Hide resolved
const forkedFromElm = select<HTMLElement>('.fork-flag:not(.rgh-forked) a');
jerone marked this conversation as resolved.
Show resolved Hide resolved
if (forkedFromElm) {
const forkedRepo = forkedFromElm.getAttribute('href')!.substring(1);
storeCache(forkedRepo, currentRepo);
}
}

// Check for opening the fork dialog.
function onForkDialogOpened(): void {
jerone marked this conversation as resolved.
Show resolved Hide resolved
const forkDialog = select<HTMLElement>('details-dialog[src*="/fork"]')!;
Copy link
Member

Choose a reason for hiding this comment

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

All the feedback I sent before applies to other parts of the file as well. I can’t review every single line. This PR has been sloppy. Please clean it up and open a new PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Webpack compiles, lint succeeds, the PR does exactly what it should do, in the way you specified it should be done, without code style guide. I try to follow other features syntax, but you have a very opinionated way of writing code. I'm not saying that that is bad, and the feedback is great.

See #2153

Copy link
Member

Choose a reason for hiding this comment

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

I haven't followed the review here, but what @bfred-it means is that when he reviews some change, and you fix it, you should look for other places where the same fix or similar fix could be applied. You should also look if there's any way to improve/simplify the code after the changes.

const forkFragment = select<HTMLElement>('include-fragment', forkDialog)!;
forkFragment.addEventListener('load', () => onFragmentLoaded(forkDialog));
}

// Event called when fork dialog is opened.
function onFragmentLoaded(parent: HTMLElement): void {
jerone marked this conversation as resolved.
Show resolved Hide resolved
removeAllHtml();
jerone marked this conversation as resolved.
Show resolved Hide resolved

const repo = getOriginalRepo();
const forks = select.all<HTMLElement>('.octicon-repo-forked', parent).map(forkElm => {
const fork = forkElm.parentNode!.textContent!.trim();
appendHtml(fork);
return fork;
});

storeCache(repo, ...forks);
}

// Get the original repo, by checking if we are already on a fork.
function getOriginalRepo(): string {
let repo = currentRepo;

const forkedFromElm = select<HTMLElement>('.fork-flag:not(.rgh-forked) a');
if (forkedFromElm) {
jerone marked this conversation as resolved.
Show resolved Hide resolved
repo = forkedFromElm.getAttribute('href')!.substring(1);
}

return repo;
}

// Get cache and sort it.
async function getCache(repo: string): Promise<string[]> {
const currentUser = getUsername();
const repoKey = key(repo);
jerone marked this conversation as resolved.
Show resolved Hide resolved
const cached = await cache.get<string[]>(repoKey) || [];
cached.sort((a, b) => {
let order = a.localeCompare(b);
if (a.startsWith(`${currentUser}/`)) {
order -= 100;
jerone marked this conversation as resolved.
Show resolved Hide resolved
}

return order;
});
return Promise.resolve(cached);
jerone marked this conversation as resolved.
Show resolved Hide resolved
}

// Save forks to cache.
jerone marked this conversation as resolved.
Show resolved Hide resolved
async function storeCache(repo: string, ...forks: string[]): Promise<void> {
const repoKey = key(repo);
const cached = await cache.get<string[]>(repoKey) || [];
for (const fork of forks) {
if (!cached.includes(fork)) {
cached.push(fork);
}
}

await cache.set<string[]>(repoKey, cached, 10);
}

// Remove the HTML created.
function removeAllHtml(): void {
const forks = select.all<HTMLElement>('.rgh-forked');
for (const fork of forks) {
fork.remove();
}
}

// Create the HTML.
function appendHtml(fork: string): void {
const pageHeader = select<HTMLElement>('.pagehead h1.public')!;
pageHeader.append(
jerone marked this conversation as resolved.
Show resolved Hide resolved
<span className={'fork-flag rgh-forked'} data-repository-hovercards-enabled>
<span className={'text'}>forked to&nbsp;
jerone marked this conversation as resolved.
Show resolved Hide resolved
<a data-hovercard-type="repository" data-hovercard-url={`/${fork}/hovercard`} href={`/${fork}`}>
jerone marked this conversation as resolved.
Show resolved Hide resolved
{fork}
</a>
</span>
</span>
);
}

// Create the cache key.
function key(repo: string): string {
return `forked-to:${repo}`;
}

features.add({
id: 'forked-to',
description: 'Add link to forked repo below the original',
include: [
features.isRepo
],
load: features.onAjaxedPages,
init
});
2 changes: 2 additions & 0 deletions source/libs/page-detect.ts
Expand Up @@ -89,6 +89,8 @@ export const isRepoSettings = (): boolean => /^settings/.test(getRepoPath()!);

export const isRepoTree = (): boolean => isRepoRoot() || /^tree\//.test(getRepoPath()!);

export const isOwnRepo = (): boolean => isRepo() && select.exists('.reponav-item[href$="/settings"]');

export const isSingleCommit = (): boolean => /^commit\/[0-9a-f]{5,40}/.test(getRepoPath()!);

export const isSingleFile = (): boolean => /^blob\//.test(getRepoPath()!);
Expand Down