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

Fix the issue that percentage text didn't display on Progress.Circle #117

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

azurechen
Copy link

No description provided.

@yarism
Copy link

yarism commented Dec 9, 2018

@oblador could you merge this? Have the same issue with percentage text not showing up. Tried this and it works.

@oblador
Copy link
Owner

oblador commented Dec 9, 2018

Problem with this PR is that it uses a private property that doesn't work when using native driver for example. The examples people have posted in the past were using the component incorrectly, do you have a simple example to reproduce it I'd be happy to fix it :-)

@yarism
Copy link

yarism commented Dec 9, 2018

Ah maybe I did it wrong then.

<Progress.Circle 
      progress={50/100} 
      size={120}
      color='white'
      showsText={true}
      textStyle={{ fontSize: 36 }}
      borderWidth={0}
      thickness={10}
      unfilledColor={'rgba(255, 255, 255, 0.2)'}
/>

Should it have been 0.5 instead and not an expression?

@Slooowpoke
Copy link

Slooowpoke commented Dec 14, 2018

The example here works. But I have the same issue as #67 Is there something wrong with how this is used?

I did just notice that the example works perfectly fine outside of a FlatList component! But if it is in an item passed in via renderItem it fails to update the text progress and it looks as though it isn't updating the animation at all like #97.

I have a small example, the first Progress.Circle utilises your example code (and runs fine) then there is a FlatList with two Progress circles which do not show an updated text and the animation feels very jerky. I can investigate further later today but I thought it might help.

<Progress.Circle
    textStyle={{ fontSize: 10 }}
    showsText={true}
    progress={this.state.progress}
    size={40}
    color='rgba(0, 122, 255, 1)'
    thickness={2}/>

<FlatList
data={[{text: "test", progress: 0.5}, {text: "test", progress: 0.8}]}
renderItem={({item}) => 
<View>
    <Progress.Circle
        textStyle={{ fontSize: 10 }}
        showsText={true}
        progress={item.progress}
        size={40}
        color='rgba(0, 122, 255, 1)'
        thickness={2}/>
</View>}
/>

Seems the similarity between my issue and this are they both use PureComponents.

Using legacyImplementation on my flatlist makes the animation work, not the best option but it works.

@cmmartin
Copy link

cmmartin commented Jan 22, 2019

@oblador When animated={true}, the value in the label will always be 0 until it changes, because of this line.

The0 initial value is never overridden until it's event listener fires here which doesn't happen unless the value actually changes.

I thought we could simply replace this in the constructor...

this.progressValue = 0;

with

this.progressValue = props.progress;

However, since your higher order component is converting props.progress to an AnimatedValue instance, we no longer have any access to its actual initial value unless we access its "private" properties like...

this.progressValue = props.animated ? props.progress._startingValue : props.progress;

@azurechen
Copy link
Author

So how can we fix the private property problem? I really want somebody merge this pr.

Copy link

@JowelTisso JowelTisso left a comment

Choose a reason for hiding this comment

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

This PR is working when the animated is true, but when the animated is false it shows NaN

@thulioxavier
Copy link

A temporary solution that I'm using is using a Timeout in a useEffect with a time of "1000", it's not the most suitable but for simpler uses it works well.

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.

7 participants