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

Better quality Github shortcut logo #193

Merged
merged 5 commits into from
Nov 19, 2024
Merged

Better quality Github shortcut logo #193

merged 5 commits into from
Nov 19, 2024

Conversation

prem-k-r
Copy link
Collaborator

image
image
to
image
image

@prem-k-r prem-k-r marked this pull request as draft November 19, 2024 04:43
@prem-k-r prem-k-r marked this pull request as ready for review November 19, 2024 05:17
@itz-rj-here
Copy link
Collaborator

It would be better if you use svg code :)

@prem-k-r

This comment was marked as outdated.

@prem-k-r prem-k-r marked this pull request as draft November 19, 2024 08:31
@prem-k-r prem-k-r marked this pull request as ready for review November 19, 2024 09:16
@prem-k-r
Copy link
Collaborator Author

It would be better if you use svg code :)

Done

Copy link
Collaborator

@itz-rj-here itz-rj-here left a comment

Choose a reason for hiding this comment

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

Great :)

@XengShi
Copy link
Owner

XengShi commented Nov 19, 2024

I don't think we need that

@XengShi
Copy link
Owner

XengShi commented Nov 19, 2024

System will check unnecessarily [ if (hostname === "github.com") ] for every website

@prem-k-r
Copy link
Collaborator Author

Is that so? This is for shortcut icon, not the website favicon though

@prem-k-r prem-k-r closed this Nov 19, 2024
@prem-k-r prem-k-r deleted the patch-3 branch November 19, 2024 11:59
@prem-k-r prem-k-r restored the patch-3 branch November 19, 2024 11:59
@prem-k-r prem-k-r reopened this Nov 19, 2024
@prem-k-r
Copy link
Collaborator Author

When a new tab opens or a page refreshes, it fetches the shortcut icons from the web each time. I believe a simple if statement shouldn't be a significant burden.

@prem-k-r
Copy link
Collaborator Author

Instead, it's a good thing as it skips fetching from the web if the domain is github.com, allowing it to work offline as well. Isn't?

@itz-rj-here
Copy link
Collaborator

Instead, it's a good thing as it skips fetching from the web if the domain is github.com, allowing it to work offline as well. Isn't?

yeah

@itz-rj-here
Copy link
Collaborator

Can we get the new release :)?

@XengShi
Copy link
Owner

XengShi commented Nov 19, 2024

Instead, it's a good thing as it skips fetching from the web if the domain is github.com, allowing it to work offline as well. Isn't?

OK then, I may misunderstood.

@XengShi XengShi merged commit b13a8ee into XengShi:main Nov 19, 2024
@prem-k-r prem-k-r deleted the patch-3 branch November 19, 2024 16:52
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.

3 participants