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

default-branch-button on forks: add secondary button to see folder on source repo #2985

Closed
fregante opened this issue Apr 8, 2020 · 15 comments · Fixed by #3230
Closed

default-branch-button on forks: add secondary button to see folder on source repo #2985

fregante opened this issue Apr 8, 2020 · 15 comments · Fixed by #3230

Comments

@fregante
Copy link
Member

fregante commented Apr 8, 2020

  1. Visit https://github.com/fregante/refined-github/blob/incremental-tag-changelog-link/source/background.ts
  2. There should be a << link to https://github.com/sindresorhus/refined-github/blob/master/source/background.ts

Example: (ignore the arrow)

How:

  1. Edit the get-default-branch function to accept a user/repo string (and it should only use parseBranchFromDom if the string matches the current page)
  2. Add the button on forks using the CSS code here: Default branch link: add support for forked repos #1132

More issues semi-related to this one: #1115 #1168 #1557

@yakov116
Copy link
Member

yakov116 commented Jun 12, 2020

I just realized that Unrelated Feature 😄 it exactly this!

Can I have a go for it?

Played around:

Version 1
image

Version 2
image

Version 3
image

@fregante
Copy link
Member Author

Version 4

No extra UI

@fregante
Copy link
Member Author

Edit the get-default-branch function to accept a user/repo string (and it should only use parseBranchFromDom if the string matches the current page)

This is still valid and should be read.

@yakov116
Copy link
Member

Part of default branch or its own feature?

@fregante
Copy link
Member Author

fork-source-link-same-view

@fregante

This comment has been minimized.

@yakov116
Copy link
Member

yakov116 commented Jun 12, 2020

I think so, it kept it very simple and it follows the way we did it for forked to.
However its your call chief

@yakov116
Copy link
Member

Edit the get-default-branch function to accept a user/repo string (and it should only use parseBranchFromDom if the string matches the current page)

This is still valid and should be read.

i lost you with this one, how would this be different that using HEAD?

@muescha
Copy link
Contributor

muescha commented Jun 12, 2020

Variant 5:

Bildschirmfoto 2020-06-12 um 15 56 32

or with master branch:

Bildschirmfoto 2020-06-12 um 15 59 37

then it is more clear that i can jump to a specific file or i can use the breadcrump higher directory to jump to this directory

because i think the link forked from ..../.... gets a blind spot because i tried it many times, but then i see this link is useless for me in this case to jump to the same file...

@yakov116
Copy link
Member

gets a blind spot because i tried it many times

Happened to me today while trying out the feature.

In other words you dont realize you changed repo's since the page looks the same.

@fregante what if we add it any only alt?

@yakov116
Copy link
Member

yakov116 commented Jun 12, 2020

I made a POC
https://github.com/yakov116/refined-github/tree/fork-source-link-same-view

Variant 5:

I think that will get too noisy on long paths

@muescha
Copy link
Contributor

muescha commented Jun 12, 2020

long path also in a much bigger font above the file, so it is the same "noice" as the file name above the file, but in very small size

@fregante
Copy link
Member Author

fregante commented Jun 12, 2020

I made a POC

This feature is 1 line long. No delegate needed.

const forkSource = select('fork source link')!;
forkSource.pathname = createLink(forkSource.textContent);

... ish, I think you'll need GitHubURL to replace the branch as well

@yakov116
Copy link
Member

@fregante
Copy link
Member Author

If we change the pathname we mess up this line

export function getForkedRepo(): string | undefined {
- 	return select<HTMLAnchorElement>('.fork-flag a')?.pathname.slice(1);
+ 	return select<HTMLAnchorElement>('.fork-flag a')?.pathname.slice(1).split('/', 2).join('/');
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants