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 source #2153

Merged
merged 12 commits into from Jul 3, 2019

Conversation

Projects
None yet
4 participants
@jerone
Copy link
Contributor

commented Jun 13, 2019

Description

Add links to the forked repos below the original repo. For users and organizations.

Closes

Closes #1129
Supersedes attempt 1 #1108
Supersedes attempt 2 #2105
Supersedes attempt 3 #2141

Screenshots

Original repo:
forked-to

User forked repo:
image

Organization forked repo:
image

Test

  1. Open any repo and click on the Fork button.
  2. Fork the repo.
  3. Go back to the original repo.
  4. Below the repo name, a link to your fork will presented.
@jerone

This comment has been minimized.

Copy link
Contributor Author

commented Jun 13, 2019

@bfred-it Clean up even more; hope it satisfies. Feel free to edit at your will.

Show resolved Hide resolved source/libs/page-detect.ts Outdated
Show resolved Hide resolved source/features/forked-to.tsx Outdated
@bfred-it

This comment has been minimized.

Copy link
Collaborator

commented Jun 23, 2019

Not a good idea to include the full issue number in the branch name, this happens: 😅

Screenshot 2019-06-23 at 15 41 55

@jerone

This comment has been minimized.

Copy link
Contributor Author

commented Jun 23, 2019

Will try to remember

@sindresorhus

This comment has been minimized.

Copy link
Owner

commented Jul 1, 2019

The implementation looks good to me. Can you mention the feature in the readme with a link to a screenshot?

@jerone

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2019

@sindresorhus commented on Jul 1, 2019, 8:46 AM GMT+2:

The implementation looks good to me. Can you mention the feature in the readme with a link to a screenshot?

Done.

Show resolved Hide resolved readme.md Outdated

@sindresorhus sindresorhus merged commit e90503f into sindresorhus:master Jul 3, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@sindresorhus

This comment has been minimized.

Copy link
Owner

commented Jul 3, 2019

Thanks for contributing this feature 🙌

sindresorhus added a commit that referenced this pull request Jul 3, 2019

Add link to forked repo below the source (#2153)
Co-authored-by: Federico Brigante <github@bfred.it>
@bfred-it

This comment has been minimized.

Copy link
Collaborator

commented Jul 3, 2019

<3

@jerone jerone deleted the jerone:#1129-forked-2 branch Jul 3, 2019

@1138-4EB

This comment has been minimized.

Copy link

commented Jul 8, 2019

@jerone, first off, thanks for contributing this feature! ❤️

I'm using Firefox Multi-Account Containers, to handle two different GitHub accounts. However, this new feature ignores 'container isolation'. I.e., if the same repo is cloned in both accounts, I see both of them listed when I visit the original repo. I would expect a single fork to be shown instead, the one that corresponds to the account that is logged in. More precisely:

  • No container: logged in with user_a, which does NOT belong to org.
  • Container Personal: logged in with user_b, which does belong to org.

When visiting orig/repo, I see:

  • forked to user_a/repo
  • forked to org/repo

Instead, I would expect to see forked to user_a/repo only, when browsing in 'No container', and forked to org/repo only when browsing in 'Personal'. This is because following the link to the fork does not change the container. The user needs to explicitly right-click to open it in the corresponding container.

Is this expected by design? Or is it worth a separate issue?

@jerone

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2019

@1138-4EB Interesting issue. You are encountering this issue, because all forks are saved in a cache storage. Cache is used on more features, but those are not account depended, like this feature. The solution would be to use a cache key which also contains the logged-in username.

@bfred-it @sindresorhus Is multiple accounts something we want to support?

@bfred-it

This comment has been minimized.

Copy link
Collaborator

commented Jul 8, 2019

@jerone this can probably be solved by including the current username in the cache key itself, e.g. forked-to:<currentuser>:<user>/<repo>

@jerone

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2019

@bfred-it Yes, exactly like I said 😄

The solution would be to use a cache key which also contains the logged-in username.

@1138-4EB

This comment has been minimized.

Copy link

commented Jul 8, 2019

@bfred-it, @jerone, do you mean in https://github.com/sindresorhus/refined-github/blob/master/source/features/forked-to.tsx#L8? Would it be forked-to:${currentuser}:${user}/${repo}?

@jerone

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2019

@1138-4EB Yeah. I'm testing right now. Will send PR soon.

@jerone

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2019

PR here: #2215

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.