sissbruecker / linkding

Self-hosted bookmark manager that is designed be to be minimal, fast, and easy to set up using Docker.
MIT License
5.33k stars 261 forks source link

Add support for bookmark thumbnails #721

Closed vslinko closed 2 months ago

vslinko commented 2 months ago

Hello!

I want to add support for bookmark thumbnails. I've made a rough implementation, but I'm not sure I'm going in the right direction. please tell me what else I should consider or change in the code.

todo:

sissbruecker commented 2 months ago

Thanks for your effort, looks promising but I'll have to take some time to think about how this should work. Some initial thoughts:

I'll try to take a closer look soon, no need to do any further changes to the PR for now. I might also decide to make some changes directly myself if it turns out to be low effort.


Bildschirmfoto 2024-05-05 um 20 39 55

vslinko commented 2 months ago

Having two fields for storing the preview image (url or downloaded image) feels a bit weird. I can see the benefit though, as that allows showing the preview image right in the form after loading website metadata. And downloading the image later helps in case the website goes offline. Though for a start, maybe just storing the URL would be enough, would also make the PR a bit smaller and easier to review.

I started with one column, replacing the remote URL with the local URL after downloading. But to do that you have to do schema checks for the URL in many places in the code, which didn't seem as convenient as keeping two links. But yeah, you could keep one field.

But anyways I see it as important to be able to store images locally, as it has many advantages:

Perhaps you can make two settings: enabling the preview viewing feature, and enabling the preview download feature, which are independent of each other.

Not sure if the image really needs to show up in the create / edit form. It's kind of neat, but also takes up a lot of space.

Agree

The bookmark list is kind of hard to read at the moment. Having the images at the start of each row makes it harder to distinguish individual bookmarks in case some of them are missing a preview image. It's also very noisy - preview and favicon are next to each other, preview images have different sizes. I think it would be better to put the images at the end of each line, give them a fixed size, and make each image cover the full size. Quick prototype below. It also takes up a lot of space on mobile, I would consider hiding the preview there.

Agree

There needs to be some way to load preview images for existing bookmarks, similar to the refresh favicons button. Makes sense to do this in a separate PR though.

Agree :)

vslinko commented 2 months ago

I would consider hiding the preview there

I think that if a person enables previews, they should work on mobiles. Or at least it should be possible to configure it.

My use case implies that I want to see previews on mobile, because I keep many YouTube links and by previews I can read faster what kind of content it is than by description.

sissbruecker commented 2 months ago

To make it a bit simpler I've removed the model field for storing the URL and only kept the field for storing the file. With that it works the same way as the favicons. That means it might take a while until the preview image shows up, basically until the respective task has been run. But that should be fine for regular usage.

Styles could maybe use some tweaking here and there, but should be good enough for a start. Long term it would be good to have an option for showing separators in the bookmark list, I think that would really help with distinguishing the bookmarks and their images from each other. For now some custom CSS could be added as a workaround.

Thanks again for the contribution, apart from some minor tweaks it was pretty complete. Note that it might take me a few weeks to release this, I currently don't have access to the machine where I cross compile the Docker images.