Page MenuHomePhabricator

Standardize image fetch/cache/fallback logic
Closed, ResolvedPublic5 Estimated Story Points

Description

  • Add SDWebImage Pod
  • Create a wrapper around SDWebImage logic (this is for implementing our own logic to fall back to the old image cache) DO NOT IMPLEMENT FALLBACK LOGIC NOW
  • Integrate new image caching component with WebView and image gallery

Fallback logic / Migration to be addressed in a future ticket: T101527


There are many instances of this pattern in the app which all have the same desired behavior:

  • If idealImage is in memory, show it immediately
  • Otherwise:
    • show fallbackImage instead (placeholder or different resolution)
    • on a background thread, fetch idealImage from the network/disk, cache it, then show it again

      Let's consolidate all of this into one abstraction so we don't need to reimplement it everywhere.

Expected results

Will it increase our velocity?

It should increase our velocity, since we'll be replacing multiple untested solutions with a single tested one.

Will it reduce risk?

Yes, by having redundant implementations we reduce the potential for one of them to do the wrong thing, and fixes applied to the shared implementation will apply across all uses.

Will it improve user happiness?

Yes, as we'll reduce more edge cases where images don't appear and we'll be smarter about how to store and pass around image data—which should save memory, disk, and CPU.

Event Timeline

BGerstle-WMF raised the priority of this task from to Needs Triage.
BGerstle-WMF updated the task description. (Show Details)
BGerstle-WMF subscribed.

Change 221738 had a related patch set uploaded (by Bgerstle):
integrate WMFImageController with image gallery

https://gerrit.wikimedia.org/r/221738

Change 221738 abandoned by Bgerstle:
integrate WMFImageController with image gallery

Reason:
folding into the patch before this one, where most review is happening. rebasing on top of revised patch not worth it

https://gerrit.wikimedia.org/r/221738

Change 220527 merged by jenkins-bot:
centralized image caching & retrieval

https://gerrit.wikimedia.org/r/220527

@Fjalapeno what are we doing with tickets targeted at 5.0? i'm not sure which column to move it to now that code review is done.

@Fjalapeno also:

  • Replace all logic throughout code to reference our new image cache to obtain images or use the UIImageView category to load images

This might not be necessary as acceptance criteria for the introduction of the image caching component. Especially since we plan to refactor the "list" user interfaces, we can also move them over to the new image fetching/caching simultaneously.