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

Unified Image Handling #3460

Open
vincode-io opened this issue Feb 11, 2022 · 2 comments
Open

Unified Image Handling #3460

vincode-io opened this issue Feb 11, 2022 · 2 comments

Comments

@vincode-io
Copy link
Member

vincode-io commented Feb 11, 2022

There are several areas regarding image handling that could be improved and enhanced. I think unified image architecture for fetching and caching images could address most of these.

Favicons

I think could do something more sophisticated than completely clearing the icon caches. Maybe we periodically check to see if the image has changed when it is accessed. Only if it has been changed would we update it and notify the UI to reload. This should remove the flashing of the default Globe for feeds with Favicons/Web Icons.

Image Zooming

I'd also like to change how we load article images to go through an internal cache of our own. We could use that image cache to look up images when zooming them. This would get rid of the current delay we have on iOS. It would also be a good time to add image zooming to the Mac as well as make the iOS app zooming more sophisticated.

See:

#647
#3349
#3354
#3360

Timeline Hero Image Thumbnail & Offline Viewing

This internal cache could be optionally loaded during the sync process. Just off the top of my head, I see 3 different modes. No caching on sync, only cache the hero image thumbnail, and full caching. Caching the hero image would let us use it quickly in the Timeline. Full caching would be used for people who want to use NNW without internet access (train commuters, etc...).

See:

#3302
#1873

Implementation Details

The heart of this work would be a replacement for ImageDownloader. It might be tempting to also try to replace some of the classes that use ImageDownloader, but there is some complicated, hard to unpack logic in those classes and they work. Let's leave those largely intact.

We would need to parse the article body to find the images and identify the hero image. The article body would also need to be changed to support a new NetNewsWire only protocol for images so that we could run them through a custom protocol handler that utilizes the ImageDownloader replacement.

The sync process would need to be enhanced to cache images depending on user preference. This might be a good time to enhance our progress indicator to show textual detail about what it is doing. That way people don't feel like they have to keep open the application when all we may be doing is trying to cache images.

Article zooming code would need to be rewritten on iOS and implemented on macOS.

We should also make sure that Unified Image Handling works for extensions and us the shared container for disk caching.

@vincode-io
Copy link
Member Author

We should evaluate Nuke with our usual skepticism to see if it makes sense include some or all of it as a dependency.

@sgraesser
Copy link

You might also want to consider the SDWebImage or Kingfisher libraries

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants