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

Android: why is Image#fadeDuration not an opt-in feature so it's consistent with iOS? #10194

Closed
gre opened this issue Oct 1, 2016 · 7 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@gre
Copy link
Contributor

gre commented Oct 1, 2016

When porting an app to Android, you notice images starts to weirdly fade in when they appear, it might not work properly if you manage yourself the "degraded loading" of images and stuff like that.
I'm wondering why this fadeDuration has been conceived to be opt-out to disable. React Native have an hardcoded default value of 300ms that you can't even override.. (what if I want to disable it completely, I can't).
Why is fadeDuration not opt-in, so you explicitely enable it? It would be consistent with Image.ios and eventually we could image iOS to support this prop too.

@lacker
Copy link
Contributor

lacker commented Dec 17, 2016

Thanks for bringing this up! I suspect this is to make things feel more native, to act like the underlying platform does. At this point making it opt-in would be a breaking change and I don't think that is worth it.

@lacker lacker closed this as completed Dec 17, 2016
@guyca
Copy link

guyca commented Dec 17, 2016

Would be nice to have this feature opt-out. I managed to prevent the fade animation by wrapping my <Image /> with a custom ViewGroupManager and setting ReactImageView.mFadeDuration to zero with reflection before the Image is added to the custom ViewGroup.

Something like this:

public class NoFadeImageWrapper extends ViewGroup {
    public NoFadeImageWrapper(Context context) {
        super(context);
    }

    @Override
    public void onViewAdded(View child) {
        if (child instanceof ReactImageView) {
            ((ReactImageView) child).setFadeDuration(0);
            ReflectionUtils.setField(child, "mIsDirty", true);
            ((ReactImageView) child).maybeUpdateView();
        }
        super.onViewAdded(child);
    }
}

ReflectionUtils.setField implementation:

public class ReflectionUtils {
    public static void setField(Object obj, String name, Object value) {
        try {
            Field field = getField(obj.getClass(), name);
            if (field == null) {
                return;
            }
            field.setAccessible(true);
            field.set(obj, value);
        } catch (Exception e) {
            e.printStackTrace();
        }
    }
}

@DylanVann
Copy link
Contributor

fadeDuration is a prop on Image for android now. This is not documented.

@gre
Copy link
Contributor Author

gre commented Jan 29, 2017

@DylanVann yeah, it's possible to override fadeDuration, but in this issue I was just asking why the default is 300ms where I think it should be 0ms if we ever want iOS and Android to be consistent.
the workaround is to define your own Image and override the defaults but I wish there would be a better way to set that default globally at least.

@DylanVann
Copy link
Contributor

@gre yeah I agree it should probably be consistent. Just mentioning that I don't think @guyca 's solution is necessary.

Just use:

<Image fadeDuration={0} {...otherProps} />

@guyca
Copy link

guyca commented Jan 29, 2017

@DylanVann Thanks a lot! Obviously that was a hack since I wasn't aware of fadeDuration

@sahrens
Copy link
Contributor

sahrens commented Dec 5, 2017

It's generally better to match platform standards rather than try to be pixel identical, so this is how we try to make the defaults. But of course you should also pay close attention to the design on both platforms and dial them in the way you want. For high quality multi-platform apps, only about 85-90% of code is shared, and the remaining 10-15% is needed to make the app conform to platform specifics.

@facebook facebook locked as resolved and limited conversation to collaborators Jul 19, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

7 participants