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

[#2128] Fix Blurry Favicon #2129

Merged
merged 5 commits into from
Mar 2, 2024
Merged

Conversation

sopa301
Copy link
Contributor

@sopa301 sopa301 commented Feb 19, 2024

Fixes #2128

Proposed commit message

Fix Blurry Favicon on Reports

The favicon looks blurry on generated reports.

Let's replace this with a clearer image.

Other information

To view the new image, clear the cache.

@sopa301 sopa301 force-pushed the 2128-blurry-favicon branch 2 times, most recently from 4697bc4 to e1af162 Compare February 19, 2024 14:23
@sopa301 sopa301 marked this pull request as ready for review February 19, 2024 15:25
@sopa301 sopa301 marked this pull request as draft February 19, 2024 15:27
@sopa301 sopa301 marked this pull request as ready for review February 19, 2024 15:37
@sopa301 sopa301 requested a review from a team February 19, 2024 16:05
Copy link
Member

@ckcherry23 ckcherry23 left a comment

Choose a reason for hiding this comment

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

Thanks for taking this up @sopa301!

Upon some research, it seems like various sources online have differing opinions on what size(s) of favicons should be used for websites.

  • While the original size we used (16x16) is considered one of the standard sizes that should work well for most browsers, it seems like larger sizes such as the 200x200 we are using now can be scaled down correctly most of the time.
  • Also, our MarkBind docs site uses the same 200x200 favicon.ico file as the site header.

Considering this, and that this will increase the clarity of the favicon, I think the larger .ico file should be okay.

@ckcherry23 ckcherry23 requested a review from a team February 21, 2024 08:53
Copy link
Contributor

@asdfghjkxd asdfghjkxd left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@MarcusTXK MarcusTXK left a comment

Choose a reason for hiding this comment

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

LGTM, I agree the 200 x 200 ico is okay

@ckcherry23 ckcherry23 merged commit 5140861 into reposense:master Mar 2, 2024
10 checks passed
Copy link
Contributor

github-actions bot commented Mar 2, 2024

The following links are for previewing this pull request:

@sopa301 sopa301 changed the title Fix Blurry Favicon [#2128] Fix Blurry Favicon Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

favicon blurry in generated reports
4 participants