-
Notifications
You must be signed in to change notification settings - Fork 3
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
Favicon loading is delaying extension UI #54
Comments
We can do image serialization (for caching) using FileReader.readAsDataURL() along with the fetch API: async function loadImageToDataURL(url)
const response = await fetch(url);
const blob = await response.blob();
return new Promise((resolve, reject) => {
const reader = new FileReader()
reader.onloadend = () => resolve(reader.result);
reader.onerror = () => reject();
reader.readAsDataURL(blob);
});
} reference: https://stackoverflow.com/a/20285053/5425899 "How can I convert an image into Base64 string using JavaScript?" |
#88 Fixes the issue where loading of favicon images can delay the loading of the extension UI itself. I didn't see any deformed images during testing. I don't think I've seen a stretched favicon since the hackathon period, maybe it's no longer an issue? Are we satisfied with the fix in #88 or do we want to also cache images so they display instantly? |
I also haven't seen this problem since the issue was created, so I agree we can assume it's not an issue anymore.
I think the fix in #88 is sufficient to address the UX problem and caching the favicons can be a future enhancement (opened #89). I'll update the title of this issue to reflect what we're solving with the PR. |
We load the favicon image into the top site circle when the extension is opened. This can result in the image being deformed or not loaded at all. On top of that, the loading of the image can delay the loading of the extension UI itself, which is bad UX. We should cache the image or otherwise may the favicon loading more efficient.
The below problems were discovered in #48.
The text was updated successfully, but these errors were encountered: