Skip to content

Conversation

@drash-course
Copy link

@drash-course drash-course commented Jun 6, 2019

I found a small bug in Circle.js; when I update the progress from 0 to 0.05 the progress bar grows to 5% but the text continues to display "0%". Further updates of the component do not show the problem.

Repro

Initialise the component with:

<Circle
  progress={0}
  color="#564"
  size={55}
  showsText
  strokeCap="round"
  style={s.progressCircle}
/>

Then update it:

<Circle
  progress={0.05}
  color="#564"
  size={55}
  showsText
  strokeCap="round"
  style={s.progressCircle}
/>

Proposed fix

It looks like the listener that is registered in componentWillMount() sets this.progressValue too late, after the component has updated. A simple solution is to stop using this.progressValue and instead read the internal value inside this.props.progress (an instance of Animated.Value). The race condition is therefore avoided.

Further improvements

I'm not sure if the componentDidMount() listener is still useful. It looks like it was there to work around this particular bug. I left it because it may be needed in some other case I'm not aware of.

Feel free to amend the merge request.

@drash-course
Copy link
Author

drash-course commented Jun 6, 2019

I discovered that my fix is similar to another PR. Using the getter method is more proper. There is no risk that the value will be driven by the native driver since it's not provided as part of an animation but set from JS as part of a parent component update.

@rphlmr
Copy link

rphlmr commented Jun 23, 2019

Do you think that will solve the "unfilledColor" bug ? (when progress >= 1 and go done, unfilledColor disappear)

@drash-course
Copy link
Author

You should try it out, I don't know about this bug unfortunately.

@Pei116
Copy link

Pei116 commented Mar 31, 2020

Can we get this one merged? The issue's 100% reproducable and needs to be fixed. @oblador

@parwan-as
Copy link

Can this pull request be merged? Thanks

@oblador
Copy link
Owner

oblador commented Jul 10, 2021

Reading internal properties of the Animated value is not the way to go as it is not a public API and is not available when using stuff like native driver.

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.

6 participants