Emoji Favicons for bookmarks #90
Conversation
|
Hello, thanks for contributing! I didn't say this in the issue, I should have, but my preferred way of doing this is to actually write the favicon to the bookmarks file. Like when you press Ctrl-D to create a bookmark for the first time, the default text starts with the favicon for the page if it exists, and a space. And if the favicon for the page changes, you can delete the bookmark and make a new one, or change it yourself. This seems like a simpler solution that allows the user to control what the bookmarks look like too. This is basically your solution 1, "persist the cache".
The main problem with this that I see is that since the cache is new on startup, usually most of the bookmarks won't have favicons at all. You'd have to visit all your bookmarked sites and never close Amfora for them to display the way you'd expect. What do you think of my solution? Could you change your PR to implement it? Let me know. |
|
Yes, that makes sense! That was one solution I thought of but forgot to write it down in my list. I’ll implement the change after work today if i have time. It seems like the only disadvantage is if pages change their favicon or add one where there wasn’t one already but it also has the added advantage of users being able to delete/edit favicons for certain pages if they want
Alex
… On Sep 16, 2020, at 9:40 AM, makeworld ***@***.***> wrote:
Hello, thanks for contributing!
I didn't say this in the issue, I should have, but my preferred way of doing this is to actually write the favicon to the bookmarks file. Like when you press Ctrl-D to create a bookmark for the first time, the default text starts with the favicon for the page if it exists, and a space. And if the favicon for the page changes, you can delete the bookmark and make a new one, or change it yourself. This seems like a simpler solution that allows the user to control what the bookmarks look like too. This is basically your solution 1, "persist the cache".
Load icon only if it is already saved in the cache
The main problem with this that I see is that since the cache is new on startup, usually most of the bookmarks won't have favicons at all. You'd have to visit all your bookmarked sites and never close Amfora for them to display the way you'd expect.
What do you think of my solution? Could you change your PR to implement it? Let me know.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
Ok, glad you agree! Thanks. |
|
OK, I think I have a change does what you're looking for -- let me know what you think! |
|
Thanks for adding this! Looks mostly good, but one bug I noticed was that the favicon gets added twice if I try to change the bookmark. For example, if you visit a site with a favicon, and then create a bookmark, the default bookmark text is Maybe you could add code that checks if the bookmark already starts with the current site's favicon, and only prepend the favicon if it doesn't? |
|
Should be fixed now! However, I realized there seems to be a bug with updating bookmarks -- created a separate issue here: #92 |
|
Looks like the original issue I had was fixed. Thanks for this PR! |
|
No problem! I’ll try and keep an eye on this repo and contribute when I have a chance
Alex
… On Sep 18, 2020, at 2:44 PM, makeworld ***@***.***> wrote:
Looks like the original issue I had was fixed. Thanks for this PR!
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
Hi! I decided I want to help contribute to this project when I have time, and I thought this would be a good issue to get started with -- #69
This isn't a perfect solution -- it just pulls from the cache right now if it's there. I can think of three solutions to this problem:
My solution is partway towards a solution based on 1. which seems to me like the best approach. What do you think?
Thanks for making this tool!