deanhet / react-native-text-ticker

React Native Text Ticker/Marquee Component
MIT License
444 stars 75 forks source link

Upgrade to 0.17.0 crashes app on Android #36

Closed Fouppy closed 5 years ago

Fouppy commented 5 years ago

Description

After upgrading to 0.17.0 from 0.16.0, I'm getting these crashes as soon as a text ticker is rendered (as you can see after, it's before it even ticks as we use methods to start and stop the animation):

image

Usage

// @flow
import * as React from 'react';
import { Easing } from 'react-native';
import TextTicker from 'react-native-text-ticker';
import styled from 'styled-components/native';

import { colors } from 'config/theme';

// -----------------------------------------------------------------------------

type Props = {
    data: string[],
    isPlaying: boolean,
    onPressHashtag: (hashtag: string) => boolean | void,
    uuid: string
};

type State = {
    loopDuration: number // ms
};

// -----------------------------------------------------------------------------

class ScrollingHashtags extends React.Component<Props, State> {
    textTicker = TextTicker;

    state = {
        loopDuration: 0
    };

    componentDidMount() {
        this.getLoopDuration();
    }

    componentDidUpdate(prevProps: Props) {
        const { data: prevData, isPlaying: prevIsPlaying } = prevProps;
        const { data, isPlaying } = this.props;

        if (prevData !== data) this.getLoopDuration();

        if (prevIsPlaying !== isPlaying) {
            if (isPlaying) this.textTicker.startAnimation();
            else this.textTicker.stopAnimation();
        }
    }

    // Multiply the hashtags strings length by 200ms
    getLoopDuration = () => {
        // +2 is for # and space chars
        const stringLength: number = this.props.data.reduce((a, v) => a + v.length + 2, 0);

        this.setState({ loopDuration: stringLength * 200 });
    };

    onPressHashtag = (hashtag: string) => this.props.onPressHashtag(hashtag);

    renderHashtag = (hashtag: string) => (
        <Hashtag key={`${this.props.uuid}_${hashtag}`} onPress={() => this.onPressHashtag(hashtag)}>
            #{hashtag}{' '}
        </Hashtag>
    );

    render() {
        const textTickerProps = {
            bounce: false,
            duration: this.state.loopDuration,
            easing: Easing.linear,
            marqueeOnMount: false, // disable auto start
            ref: (c: any) => {
                this.textTicker = c;
            },
            repeatSpacer: 0,
            scroll: false // prevent the user from grabbing and scrolling
        };

        return (
            <Container>
                <TextTicker {...textTickerProps}>{this.props.data.map(this.renderHashtag)}</TextTicker>
            </Container>
        );
    }
}

// -----------------------------------------------------------------------------

export default ScrollingHashtags;

// -----------------------------------------------------------------------------

const Container = styled.View`
    flex-grow: 0;
    flex-shrink: 1;
    margin-bottom: 4px;
    margin-right: 5px;
`;

const Hashtag = styled.Text`
    color: ${colors.primary};
    font-size: 16px;
    font-weight: 500;
`;

Environment

React Native Environment Info:
    System:
      OS: macOS 10.14.5
      CPU: (4) x64 Intel(R) Core(TM) i7-6567U CPU @ 3.30GHz
      Memory: 610.24 MB / 16.00 GB
      Shell: 5.3 - /bin/zsh
    Binaries:
      Node: 12.4.0 - /usr/local/bin/node
      Yarn: 1.16.0 - /usr/local/bin/yarn
      npm: 6.9.0 - /usr/local/bin/npm
      Watchman: 4.9.0 - /usr/local/bin/watchman
    SDKs:
      iOS SDK:
        Platforms: iOS 12.2, macOS 10.14, tvOS 12.2, watchOS 5.2
      Android SDK:
        API Levels: 23, 25, 26, 27, 28
        Build Tools: 23.0.1, 23.0.3, 25.0.0, 25.0.1, 25.0.2, 25.0.3, 26.0.0, 26.0.1, 26.0.2, 27.0.0, 27.0.3, 28.0.2, 28.0.3
        System Images: android-23 | Intel x86 Atom_64, android-23 | Google APIs Intel x86 Atom_64, android-25 | Google APIs Intel x86 Atom_64, android-26 | Google APIs Intel x86 Atom, android-27 | Google APIs Intel x86 Atom, android-27 | Google Play Intel x86 Atom
    IDEs:
      Android Studio: 3.2 AI-181.5540.7.32.5014246
      Xcode: 10.2.1/10E1001 - /usr/bin/xcodebuild
    npmPackages:
      react: 16.8.6 => 16.8.6
      react-native: 0.59.9 => 0.59.9
    npmGlobalPackages:
      create-react-native-app: 2.0.2
      react-native-cli: 2.0.1
      react-native-git-upgrade: 0.2.7
Fouppy commented 5 years ago

As a side note, the generated snapshots from our tests change:

image

Hope this helps!

deanhet commented 5 years ago

This is peculiar. #33 was the only feature that changed between those versions. I've got a lot on right now so can't delve into it too much myself but it may be worth you reverting/playing with those lines to see if it fixes it. If that's the case I'd be happy to look over a PR. Best case scenario would be to keep the existing functionality with a fix. Mindful that crashes are far from ideal though so can always revert that commit as a last resort.

Fouppy commented 5 years ago

Yeah, I've checked the commits and can't understand why it happens. I'll try some fixes and let you know. Anyway thanks for the feedback :)

Fouppy commented 5 years ago

Ok some quick update: I was using multiple instances of your lib within a FlatList, and the culprit seemed to be some undefined results for the calculation of text and container widths.

I had no time to work on a potential fix for your lib and just removed it for the time being.

deanhet commented 5 years ago

That sounds like a feasible explanation. TextTicker measures its parent when it mounts and I imagine things start to get confused when it's inside a Virtualised list as that's doing a lot of off-screen magic to optimize rendering. I think a rewrite would have to happen for them to work together properly.

It's something I'll definitely keep in mind for the future. Will close this for now, thanks for getting back to me!

ammarRajabA commented 5 years ago

Isn't there any temporary fix for this issue ?

deanhet commented 5 years ago

It wont be as performant for sure but ScrollView instead of FlatList will probably still work. I would set the scroll prop to false too in case the pan-responders from TextTicker and the list are conflicting

deanhet commented 5 years ago

@Fouppy @ammarRajabA Can you try this PR and see if it fixes your problem? Going to test myself too but keen to hear your input.

https://github.com/deanhet/react-native-text-ticker/pull/37

ammarRajabA commented 5 years ago

@deanhet I've just tried it and it's perfect working as supposed to, many thanks.

deanhet commented 5 years ago

This issue should now be resolved with #37. Merged and released in version 0.18.0

MasoudShah commented 4 years ago

This crash happens again. I am using textTicker inside a flatList item. Current version of the lib is 1.3.0.