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.

Sign up for GitHub

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

Issue/glide fallback placeholder #7976

Merged
merged 5 commits into from
Jul 3, 2018
Merged

Issue/glide fallback placeholder #7976

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a32d216
Move ImageManager to util.image package
malinajirka Jul 2, 2018
0e0539e
Add fallback and placeholder images to ImageViews in ReaderPostList
malinajirka Jul 2, 2018
049bfb9
Checkstyle
malinajirka Jul 2, 2018
0781bdf
Use private extension functions in ImageManager
malinajirka Jul 3, 2018
2827a8d
Rename methods in ImagePlaceholderManager
malinajirka Jul 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion WordPress/src/main/java/org/wordpress/android/ui/reader/ReaderPostListFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
import org.wordpress.android.ui.ActivityLauncher;
import org.wordpress.android.ui.EmptyViewMessageType;
import org.wordpress.android.ui.FilteredRecyclerView;
import org.wordpress.android.ui.ImageManager;
import org.wordpress.android.util.image.ImageManager;
import org.wordpress.android.ui.main.BottomNavController;
import org.wordpress.android.ui.main.MainToolbarFragment;
import org.wordpress.android.ui.main.WPMainActivity;
Expand Down
35 changes: 17 additions & 18 deletions WordPress/src/main/java/org/wordpress/android/ui/reader/adapters/ReaderPostAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
import org.wordpress.android.models.ReaderPostDiscoverData;
import org.wordpress.android.models.ReaderPostList;
import org.wordpress.android.models.ReaderTag;
import org.wordpress.android.ui.ImageManager;
import org.wordpress.android.util.image.ImageManager;
import org.wordpress.android.ui.reader.ReaderActivityLauncher;
import org.wordpress.android.ui.reader.ReaderAnim;
import org.wordpress.android.ui.reader.ReaderConstants;
Expand Down Expand Up @@ -66,6 +66,7 @@
import org.wordpress.android.util.GravatarUtils;
import org.wordpress.android.util.NetworkUtils;
import org.wordpress.android.util.ToastUtils;
import org.wordpress.android.util.image.ImageType;

import java.util.HashSet;

Expand Down Expand Up @@ -388,11 +389,11 @@ private void renderXPost(int position, ReaderXPostViewHolder holder) {
}

mImageManager
.loadIntoCircle(holder.mImgAvatar, GravatarUtils.fixGravatarUrl(post.getPostAvatar(), mAvatarSzSmall),
R.drawable.ic_placeholder_gravatar_grey_lighten_20_100dp);
.loadIntoCircle(holder.mImgAvatar, ImageType.AVATAR,
GravatarUtils.fixGravatarUrl(post.getPostAvatar(), mAvatarSzSmall));

mImageManager.load(holder.mImgBlavatar, GravatarUtils.fixGravatarUrl(post.getBlogImageUrl(), mAvatarSzSmall),
R.drawable.ic_placeholder_blavatar_grey_lighten_20_40dp);
mImageManager.load(holder.mImgBlavatar, ImageType.BLAVATAR,
GravatarUtils.fixGravatarUrl(post.getBlogImageUrl(), mAvatarSzSmall));

holder.mTxtTitle.setText(ReaderXPostUtils.getXPostTitle(post));
holder.mTxtSubtitle.setText(ReaderXPostUtils.getXPostSubtitleHtml(post));
Expand Down Expand Up @@ -436,11 +437,11 @@ private void renderPost(final int position, final ReaderPostViewHolder holder) {
if (post.hasPostAvatar()) {
String imageUrl = GravatarUtils.fixGravatarUrl(post.getPostAvatar(), mAvatarSzMedium);
mImageManager.loadIntoCircle(holder.mImgAvatarOrBlavatar,
imageUrl, null);
ImageType.AVATAR, imageUrl);
holder.mImgAvatarOrBlavatar.setVisibility(View.VISIBLE);
} else if (post.hasBlogImageUrl()) {
String imageUrl = GravatarUtils.fixGravatarUrl(post.getBlogImageUrl(), mAvatarSzMedium);
mImageManager.load(holder.mImgAvatarOrBlavatar, imageUrl, null);
mImageManager.load(holder.mImgAvatarOrBlavatar, ImageType.BLAVATAR, imageUrl);
holder.mImgAvatarOrBlavatar.setVisibility(View.VISIBLE);
} else {
mImageManager.cancelRequestAndClearImageView(holder.mImgAvatarOrBlavatar);
Expand Down Expand Up @@ -469,8 +470,8 @@ private void renderPost(final int position, final ReaderPostViewHolder holder) {
holder.mFramePhoto.setVisibility(View.VISIBLE);
holder.mTxtPhotoTitle.setVisibility(View.VISIBLE);
holder.mTxtPhotoTitle.setText(post.getTitle());
mImageManager.load(holder.mImgFeatured, post.getFeaturedImageForDisplay(mPhotonWidth, mPhotonHeight),
null, ScaleType.CENTER_CROP);
mImageManager.load(holder.mImgFeatured, ImageType.PHOTO,
post.getFeaturedImageForDisplay(mPhotonWidth, mPhotonHeight), ScaleType.CENTER_CROP);
holder.mThumbnailStrip.setVisibility(View.GONE);
} else {
mImageManager.cancelRequestAndClearImageView(holder.mImgFeatured);
Expand All @@ -495,7 +496,7 @@ private void renderPost(final int position, final ReaderPostViewHolder holder) {
} else if (post.getCardType() == ReaderCardType.VIDEO) {
ReaderVideoUtils.retrieveVideoThumbnailUrl(post.getFeaturedVideo(), new VideoThumbnailUrlListener() {
@Override public void showThumbnail(String thumbnailUrl) {
mImageManager.load(holder.mImgFeatured, thumbnailUrl, null, ScaleType.CENTER_CROP);
mImageManager.load(holder.mImgFeatured, ImageType.PHOTO, thumbnailUrl, ScaleType.CENTER_CROP);
}

@Override public void showPlaceholder() {
Expand All @@ -512,8 +513,8 @@ private void renderPost(final int position, final ReaderPostViewHolder holder) {
holder.mThumbnailStrip.setVisibility(View.GONE);
titleMargin = mMarginLarge;
} else if (post.hasFeaturedImage()) {
mImageManager.load(holder.mImgFeatured, post.getFeaturedImageForDisplay(mPhotonWidth, mPhotonHeight),
null, ScaleType.CENTER_CROP);
mImageManager.load(holder.mImgFeatured, ImageType.PHOTO,
post.getFeaturedImageForDisplay(mPhotonWidth, mPhotonHeight), ScaleType.CENTER_CROP);
holder.mFramePhoto.setVisibility(View.VISIBLE);
holder.mThumbnailStrip.setVisibility(View.GONE);
titleMargin = mMarginLarge;
Expand Down Expand Up @@ -624,9 +625,8 @@ private void showDiscoverData(final ReaderPostViewHolder postHolder,

switch (discoverData.getDiscoverType()) {
case EDITOR_PICK:
mImageManager.loadIntoCircle(postHolder.mImgDiscoverAvatar,
GravatarUtils.fixGravatarUrl(discoverData.getAvatarUrl(), mAvatarSzSmall),
R.drawable.ic_placeholder_gravatar_grey_lighten_20_100dp);
mImageManager.loadIntoCircle(postHolder.mImgDiscoverAvatar, ImageType.AVATAR,
GravatarUtils.fixGravatarUrl(discoverData.getAvatarUrl(), mAvatarSzSmall));
// tapping an editor pick opens the source post, which is handled by the existing
// post selection handler
postHolder.mLayoutDiscover.setOnClickListener(new View.OnClickListener() {
Expand All @@ -641,9 +641,8 @@ public void onClick(View v) {

case SITE_PICK:
// BLAVATAR
mImageManager.load(postHolder.mImgDiscoverAvatar,
GravatarUtils.fixGravatarUrl(discoverData.getAvatarUrl(), mAvatarSzSmall),
R.drawable.ic_placeholder_blavatar_grey_lighten_20_40dp);
mImageManager.load(postHolder.mImgDiscoverAvatar, ImageType.BLAVATAR,
GravatarUtils.fixGravatarUrl(discoverData.getAvatarUrl(), mAvatarSzSmall));
// site picks show "Visit [BlogName]" link - tapping opens the blog preview if
// we have the blogId, if not show blog in internal webView
postHolder.mLayoutDiscover.setOnClickListener(new View.OnClickListener() {
Expand Down
67 changes: 37 additions & 30 deletions .../org/wordpress/android/ui/ImageManager.kt → ...dpress/android/util/image/ImageManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
package org.wordpress.android.ui
package org.wordpress.android.util.image

import android.graphics.Bitmap
import android.graphics.drawable.Drawable
import android.support.annotation.DrawableRes
import android.widget.ImageView
import android.widget.ImageView.ScaleType
import android.widget.ImageView.ScaleType.CENTER
Expand All @@ -16,17 +15,18 @@ import javax.inject.Singleton
* Singleton for asynchronous image fetching/loading with support for placeholders, transformations and more.
*/
@Singleton
class ImageManager @Inject constructor() {
class ImageManager @Inject constructor(val placeholderManager: ImagePlaceholderManager) {
@JvmOverloads
fun load(
imageView: ImageView,
imageType: ImageType,
imgUrl: String,
@DrawableRes placeholder: Int? = null,
scaleType: ImageView.ScaleType = CENTER
) {
var request = GlideApp.with(imageView.context)
.load(imgUrl)
.let { if (placeholder != null) it.fallback(placeholder) else it }
request = addFallback(request, imageType)
request = addPlaceholder(request, imageType)
request = applyScaleType(request, scaleType)
request.into(imageView)
}
Expand All @@ -40,20 +40,19 @@ class ImageManager @Inject constructor() {
}

@JvmOverloads
fun load(imageView: ImageView, imgUrl: Drawable, scaleType: ImageView.ScaleType = CENTER) {
fun load(imageView: ImageView, drawable: Drawable, scaleType: ImageView.ScaleType = CENTER) {
var request = GlideApp.with(imageView.context)
.load(imgUrl)
.load(drawable)
request = applyScaleType(request, scaleType)
request.into(imageView)
}

@JvmOverloads
fun loadIntoCircle(imageView: ImageView, imgUrl: String, @DrawableRes placeholder: Int? = null) {
val request = GlideApp.with(imageView.context)
fun loadIntoCircle(imageView: ImageView, imageType: ImageType, imgUrl: String) {
var request = GlideApp.with(imageView.context)
.load(imgUrl)
.let { if (placeholder != null) it.fallback(placeholder) else it }
.circleCrop()
request.into(imageView)
request = addFallback(request, imageType)
request = addPlaceholder(request, imageType)
request.circleCrop().into(imageView)
}

fun cancelRequestAndClearImageView(imageView: ImageView) {
Expand All @@ -79,55 +78,63 @@ class ImageManager @Inject constructor() {
}
}

private fun addPlaceholder(request: GlideRequest<Drawable>, imageType: ImageType): GlideRequest<Drawable> {
Copy link
Contributor

Choose a reason for hiding this comment

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

this might be a good usecase for a private extension function, especially since this is basically a builder pattern

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea, it looks better now!

val placeholderImageRes = placeholderManager.getPlaceholderImage(imageType)
return if (placeholderImageRes == null) request else request.placeholder(placeholderImageRes)
}

private fun addFallback(request: GlideRequest<Drawable>, imageType: ImageType): GlideRequest<Drawable> {
val errorImageRes = placeholderManager.getErrorImage(imageType)
return if (errorImageRes == null) request else request.error(errorImageRes)
}

@Deprecated("Object for backward compatibility with code which doesn't support DI")
companion object {
@JvmStatic
@Deprecated("Use injected ImageManager",
ReplaceWith("imageManager.load(imageView, imgUrl, placeholder, scaleType)",
"org.wordpress.android.ui.ImageManager"))
@JvmOverloads
"org.wordpress.android.util.image.ImageManager"))
fun loadImage(
imageView: ImageView,
imageType: ImageType,
imgUrl: String,
@DrawableRes placeholder: Int? = null,
scaleType: ImageView.ScaleType
) {
ImageManager().load(imageView, imgUrl, placeholder, scaleType)
ImageManager(ImagePlaceholderManager()).load(imageView, imageType, imgUrl, scaleType)
}

@JvmStatic
@Deprecated("Use injected ImageManager",
ReplaceWith("imageManager.load(imageView, bitmap, placeholder, scaleType)",
"org.wordpress.android.ui.ImageManager"))
ReplaceWith("imageManager.load(imageView, bitmap, scaleType)",
"org.wordpress.android.util.image.ImageManager"))
@JvmOverloads
fun loadImage(imageView: ImageView, bitmap: Bitmap, scaleType: ImageView.ScaleType = CENTER) {
ImageManager().load(imageView, bitmap, scaleType)
ImageManager(ImagePlaceholderManager()).load(imageView, bitmap, scaleType)
}

@JvmStatic
@Deprecated("Use injected ImageManager",
ReplaceWith("imageManager.load(imageView, drawable, placeholder, scaleType)",
"org.wordpress.android.ui.ImageManager"))
ReplaceWith("imageManager.load(imageView, drawable, scaleType)",
"org.wordpress.android.util.image.ImageManager"))
@JvmOverloads
fun loadImage(imageView: ImageView, drawable: Drawable, scaleType: ImageView.ScaleType = CENTER) {
ImageManager().load(imageView, drawable, scaleType)
ImageManager(ImagePlaceholderManager()).load(imageView, drawable, scaleType)
}

@JvmStatic
@Deprecated("Use injected ImageManager",
ReplaceWith("imageManager.loadIntoCircle(imageView, imgUrl, placeholder)",
"org.wordpress.android.ui.ImageManager"))
@JvmOverloads
fun loadImageIntoCircle(imageView: ImageView, imgUrl: String, @DrawableRes placeholder: Int? = null) {
ImageManager().loadIntoCircle(imageView, imgUrl, placeholder)
ReplaceWith("imageManager.loadIntoCircle(imageView, imgType, imgUrl)",
"org.wordpress.android.util.image.ImageManager"))
fun loadImageIntoCircle(imageView: ImageView, imageType: ImageType, imgUrl: String) {
ImageManager(ImagePlaceholderManager()).loadIntoCircle(imageView, imageType, imgUrl)
}

@JvmStatic
@Deprecated("Use injected ImageManager",
ReplaceWith("imageManager.clear(imageView)",
"org.wordpress.android.ui.ImageManager"))
"org.wordpress.android.util.image.ImageManager"))
fun clear(imageView: ImageView) {
ImageManager().cancelRequestAndClearImageView(imageView)
ImageManager(ImagePlaceholderManager()).cancelRequestAndClearImageView(imageView)
}
}
}
26 changes: 26 additions & 0 deletions WordPress/src/main/java/org/wordpress/android/util/image/ImagePlaceholderManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package org.wordpress.android.util.image

import org.wordpress.android.R
import javax.inject.Inject
import javax.inject.Singleton

@Singleton
class ImagePlaceholderManager @Inject constructor() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I love this class 👍 Is it worth testing it? It's probably not necessary

fun getErrorImage(imgType: ImageType): Int? {
return when (imgType) {
ImageType.PHOTO -> R.color.grey_lighten_30
ImageType.VIDEO -> R.color.grey_lighten_30
ImageType.AVATAR -> R.drawable.ic_placeholder_gravatar_grey_lighten_20_100dp
ImageType.BLAVATAR -> R.drawable.ic_placeholder_blavatar_grey_lighten_20_40dp
}
}

fun getPlaceholderImage(imgType: ImageType): Int? {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe let's rename both to getPlaceholderResource since they are not really returning images?

return when (imgType) {
ImageType.PHOTO -> R.color.grey_light
ImageType.VIDEO -> R.color.grey_light
ImageType.AVATAR -> R.drawable.shape_oval_grey_light
ImageType.BLAVATAR -> R.color.grey_light
}
}
}
8 changes: 8 additions & 0 deletions WordPress/src/main/java/org/wordpress/android/util/image/ImageType.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package org.wordpress.android.util.image

enum class ImageType {
PHOTO,
VIDEO,
AVATAR,
BLAVATAR
}