✨ feat(completions): add tab favicon for better image quality #4533
Conversation
|
I can see the benefit of this. I might be misunderstanding the code, but it looks like you have pushed the problem back by setting a new size of 64px. Imagining into the future, is there a potential that someone else will make a PR to change the new 64px size to something higher? In other words, is 64px also just an arbitrary size? If so, maybe we can find a way to get the size the CSS is actually trying to use. Also, it looks like these changes might mean a 64px image is always being pulled now, even for users who use a 16px thumbnail, which would slow down the experience of the majority of existing users. I completely agree that we should fix this issue so that we can have larger icons appear sharp for the users who want it, but I'm wondering if I'm understanding the code correctly and if so, if we can improve some of the downsides. Thank you for your contribution. |
abelfubu
force-pushed
the
feature/vomnibar-results-favicon-size-setting
branch
from
a0729d2 to
d7f2ce3
Compare
| faviconUrl.searchParams.set("size", "16"); | ||
| faviconHtml = `<img class="vomnibarIcon" src="${faviconUrl.toString()}" />`; | ||
| faviconUrl.searchParams.set("size", "64"); | ||
| const src = this.favIconUrl?.startsWith("http") ? this.favIconUrl : faviconUrl.toString(); |
There was a problem hiding this comment.
If this.faviconUrl is valid and present, then for clarity, we shouldn't bother constructing and initializing the faviconUrl URL object in the preceding lines.
What caused you to add a startsWith("http") check here?
| @@ -489,6 +491,7 @@ class TabCompleter { | |||
| queryTerms, | |||
| description: "tab", | |||
| url: tab.url, | |||
| favIconUrl: tab.favIconUrl, | |||
There was a problem hiding this comment.
It seems we're passing this along because tab.favIconUrl is sometimes a higher quality image than what can be fetched from chrome's favicon API? Is so, we should mention that here in a comment, with a reference. This is the Chromium bug I found about it, and some related discussion.
Description
As many of you know, a significant number of users apply custom CSS to enhance their Vimium experience. While creating my own custom styling, I encountered a limitation with the Vomnibar: the favicon is always fetched at a 16px size. This fixed size makes resizing the icon challenging, as it can result in a blurry appearance.
This PR addresses this issue by fetching the tab's favicon URL directly when available, allowing for a higher-quality icon. If the favicon isn't available, the PR provides a fallback with an improved size, ensuring the icon looks sharp and visually appealing even when resized.