Skip to content

Fetch and use thumbnail across the app #2906

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

Merged
merged 1 commit into from
Apr 23, 2019

Conversation

maskaravivek
Copy link
Member

@maskaravivek maskaravivek commented Apr 21, 2019

Description (required)

Why we are replacing MediaWikiImageView (which extends SimpleDraweeView) with SimpleDraweeView?

  • MediaWikiImageView takes a filename and does an API call to fetch its thumbnail using an API call ie findThumbnailByFilename and then maintains a filename to thumbnail URL cache.

As the behavior of fetching the thumbnail was part of the view itself, the thumbnail is fetched every time the view is rendered(if not already in the cache). That results in an extra API call being made for every single image.

Now, as the app is consuming the imageinfo API, we already have the thumbnail URL for most of the images ie explore, search, POTD and media detail. Only contribution list doesn't have thumbnail as of now. If we implement #2904, even the contribution list will have thumbnail beforehand. Until then contribution view holder can fetch and display the thumbnail.

Fixes #2898

How?

Earlier MediaWikiImageView was used in media detail and as per the behaviour of MediaWikiImageView, it was showing thumbnails.

Fixes #2820

How?

The change in appendMediaProperties automatically gets 640 width thumbs for all media. This is in addition to the original url.

Fixes #2907

How?

Using Fresco's image controller, we are loading low and res images to a simple drawee view.

fetchAndSaveThumbnail(contribution);
}

private void fetchAndSaveThumbnail(DisplayableContribution contribution) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be removed once #2904 is in place.

@maskaravivek maskaravivek marked this pull request as ready for review April 21, 2019 17:38
@misaochan misaochan merged commit 37e9eae into commons-app:master Apr 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants