-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 repo-header-info
feature
#6900
Add repo-header-info
feature
#6900
Conversation
Renaming this feature is welcome |
); | ||
} | ||
|
||
async function addForCompact(button: HTMLButtonElement): Promise<void> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t think we should show this on mobile, there's not enough space especially under 600px
function createStarCounter(stargazerCount: number): HTMLElement { | ||
return ( | ||
<div className="starred ml-2"> | ||
<StarFillIcon className="starred-button-icon mr-1"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd probably keep it dimmed so it doesn't attract too much attention. Also because "yellow" means "starred by me" on GitHub, which we're not taking into conservation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to show "I stared" ?
I'll make it yellow when "I starred" or dimmed when "I not starred"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No let's not include that piece of information
/* | ||
Test URLs | ||
|
||
Issues: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to label each URL if there's nothing special about them. The URLs are multiple only to show that the feature works on these pages, but the pages don't differ in any meaningful way.
dc9d892
to
229c9c5
Compare
Can we also add a fork status? I think that's more important than stars. Example URL: This should be really near the lock icon as well. Possible? |
I think these are the related icons, to be used at 12px:
I think they are present in our Octicons version #6878 |
source/features/repo-header-info.tsx
Outdated
} | ||
|
||
function init(signal: AbortSignal): void { | ||
observe('header .AppHeader-context-full > nav ul', add, {signal}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM already
source/features/repo-header-info.tsx
Outdated
/* | ||
Test URLs | ||
|
||
Repository: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Repository: |
😅 Maybe zoomed with Chrome. Edit: Yes screenshot is latest image |
|
So, uhm, GitHub sometimes will show a fork icon 🤷♂️ I don't know when. I provided a demo URL, can you confirm you also see it there natively? |
readme.md
Outdated
@@ -159,6 +159,7 @@ Thanks for contributing! 🦋🙌 | |||
- [](# "unreleased-commits") 🔥 [Tells you whether you're looking at the latest version of a repository, or if there are any unreleased commits.](https://user-images.githubusercontent.com/1402241/234576563-1a0ca255-4c0d-45ae-883d-2b1aa2d7f4c1.png) | |||
- [](# "action-pr-link") 🔥 [Adds a link back to the PR that ran the workflow.](https://github-production-user-asset-6210df.s3.amazonaws.com/50487467/241645264-076a0137-36a2-4fd0-a66e-735ef3b3a563.png) | |||
- [](# "mobile-tabs") [Makes the tabs more compact on mobile so more of them can be seen.](https://user-images.githubusercontent.com/1402241/245446231-28f44b59-0151-4986-8cb9-05b5645592d8.png) | |||
- [](# "repo-header-info") [Shows repository's stargazer count on the header.](https://github.com/refined-github/refined-github/assets/50487467/747b5133-61b8-490e-b961-4e5390ea9482) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/assets/ URLs are not accepted by our parser (#6000 (comment)), you'll have to open the link and get its real URL ending with .png (already done in a commit)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welp, nope, yet another GitHub bug. https://github.com/refined-github/fork is an actual private fork, but GitHub forgot to give it the "lock" icon. We should do this too, in this feature. Feel like adding it? 😅 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably easy to implement. No need to update the screenshots
AFAIK, Making |
Why? It's a forked repo so it deserves a forked icon. That fork was made by:
So they can exist |
🙌 |
It does not show the full star count on hover like the original location. If the count is, for example, 1.1k, on hover it should say the exact amount in an HTML |
Description
details
component, is too heavy and ugly.Test URLs
Screenshot
outdated