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

Improve image loader #72

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

philippeauriach
Copy link
Contributor

@philippeauriach philippeauriach commented Aug 9, 2017

Objective : Allow developer to have more context info about image loading (for example, getting the IUser object, so we can set a placeholder with the user initials in case he has no avatar).

In every place where the library previously used an ImageLoader, it now uses a ContextImageLoader. It has three methods :

void loadImage(ImageView imageView, IDialog dialog);
void loadImage(ImageView imageView, IUser user);
void loadImage(ImageView imageView, MessageContentType.Image messageContent);

Those methods are called depending of the context.
For non breaking change and simplicity for users who do not need this, ImageLoader is now an abstract class, having the same method it had when it was an interface (non breaking change ;) ), and implementing ContextImageLoader with the same calls that were previously made :

public abstract class ImageLoader implements ContextImageLoader {

    public abstract void loadImage(ImageView imageView, String url);

    @Override
    public void loadImage(ImageView imageView, IDialog dialog) {
        loadImage(imageView, dialog.getDialogPhoto());
    }

    @Override
    public void loadImage(ImageView imageView, IUser user) {
        loadImage(imageView, user.getAvatar());
    }

    @Override
    public void loadImage(ImageView imageView, MessageContentType.Image messageContent) {
        loadImage(imageView, messageContent.getImageUrl());
    }
}

This allow developers to have fine grain control over how the images are loaded, as it is now possible to use an instance of ImageLoader, with implementing the basic loadImage(ImageView, String) AND overriding loadImage(ImageView, IUser) to provide a specific placeholder for example.

I personnaly use this to load a TextDrawable with the initials of the user in it (I included an example in the demo app).

Without breaking change : ImageLoader is now an abstract class implementing the new methods of ContextImageLoader with the values used previously : nothing change for existing implementations.
However, the user is now able to override other methods of ImageLoader if he wants to have additional data for say, the user.
@Frozmen
Copy link

Frozmen commented Sep 10, 2017

@philippeauriach, yep. nice. It will make image loading more flexible. I also faced this, and find an easy hack. For example, if you need rounded users avatar, or set some placeholder if a user hasn't avatar you can return some constant instead of image URL or add constant as a prefix to URL than removing it in ImageLoader. Maybe this will help someone.

public class Author implements IUser {

...
    public static final String AVATAR_IMAGE = "this_is_avatar";

    @Override public String getAvatar() {
       //hack to track if its avatar for circle transform later
        return avatar.small !=null ? AVATAR_IMAGE + avatar.small : NO_AVATAR;

    }
...
}
 public MessagesListAdapter<Message> adapter = new MessagesListAdapter<>(userId, new ImageLoader() {
        @Override public void loadImage(ImageView imageView, String url) {
            if (url.equals(MessageAuthor.NO_AVATAR)) {
                imageView.setImageResource(R.drawable.fake_avatar);
            } else {
              if(url.contains(MessageAuthor.AVATAR_IMAGE)){
                    url = url.replace(MessageAuthor.AVATAR_IMAGE, "");
                   ...
                    //make with avatar what you need, in my case - make it rounded
                  ...
                } else {
                    //simple load image
                    Glide.with(imageView.getContext()).load(url).into(imageView);
                }

            }
        }
    });

@philippeauriach
Copy link
Contributor Author

This is way too much of a hack for me ^^

@mario
Copy link

mario commented Jun 26, 2018

Hello,

considering that the library is not maintained and I actively use it, I have decided to fork it for the time being under the same name. Changes to naming and other practices will come in the future.

In the mean time, I will look at your PR and consider merging it.

I hope we can revive this project together.

https://github.com/mario/chatkit

Cheers,
Mario

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants