Add a third tint color to enhance the UI experience

[Monir-Shembesh]

After testing with only two tint colors the progress circle felt limited. For example, going from the color 'green' to the color 'red' at 100% produces a weird darkish 'green' color at 50%. Where as, some developers such as myself would like the 50% bar to be a yellow tint color. The solution is simple and straight forward, I have added a third tint color to enable a smoother color transition.

Reference: Traffic Light, credit card balance limit reached(green, yellow, red)

[talhaazhar]

hey @Monir-Shembesh when I was making my changes I thought of this edge-case being super useful but was too lazy to add it. Wouldn't it be more beneficial to keep tintColorSecondary and just change it to an array? This way, if someone is looking to add 5 colors in the future, they can just reuse your code!

const arrayLength = tintColorSecondary.length;
let inputRange = Array.from({length: arrayLength + 1}).map(
   (currElement, i) => i * (100 / arrayLength)
);
let outputRange = tintColorSecondary.unshift(this.props.tintColor);

[Monir-Shembesh]

Hi @talhaazhar check this implementation then?

animateColor() {

    if (!this.props.multiColor) {
      return this.props.singleColor
    }

    const colorsArry = this.props.multiColor.length;
    const inputRange = Array.from({ length: colorsArry }).map(
      (currElement, index) => index * (100 / colorsArry));

    const tintAnimation = this.state.fillAnimation.interpolate({
      inputRange,
      outputRange: this.props.multiColor
    })

    return tintAnimation

  }

so what I am aiming for here is instead of using tint color and then tintColorSecondary to achieve more than one color effect, we give the developer the option to either choose a single color or two or more colors.

before:

<AnimatedCircularProgress
                        size={width(50)}
                        width={20}
                        fill={50}
                        rotation={0}
                        lineCap={'round'}
                        tintColor={'red'}
                        tintColorSecondary={['blue', 'Yellow']}
/>

after:

<AnimatedCircularProgress
                        size={width(50)}
                        width={20}
                        fill={50}
                        rotation={0}
                        lineCap={'round'}
                        singleColor={'red'}
                        //or multiColor={['blue', 'Yellow']}
/>

The issue now is how to tell the developers that multi color prop will need minimum of two colors otherwise output range will throw an error

[talhaazhar]

Hey @Monir-Shembesh, this looks great. However, appending tintColor to the start of the array fixes this (instead of expecting the user to add it):

let outputRange = tintColorSecondary.unshift(this.props.tintColor);

This way the tintColor is always used as the first one. Also, you could rename multiColor to something like tintTransitionColors, it might sound more intuitive.

Although, the real solution would be to remove tintColor all together and just have the array color option. I don't know if that'd be too dramatic of a change for old users though.

[Monir-Shembesh]

yh unshift will solve the issue but i only wanted to keep the multiColors array. in my local repo i already remove tintColor as i displayed above. The users dont need to update to the latest package version if they dont need multiColors but if they do they can update and we can then leave a note in the README file that illustrate this change. what do you think? @talhaazhar

[talhaazhar]

I already agree with the idea of removing tintColor completely since that is the correct way to handle this update, however, the final review depends on the Repo Owner. Although it'd be nice to rename the multiColor to tintTransitionColors.

[Monir-Shembesh]

Yh I agree with tintTransitionColors, I will write something up in a bit further improve the above code and send a pull request. @talhaazhar @bartgryszko

[aboveyunhai]

Hey @Monir-Shembesh, this looks great. However, appending tintColor to the start of the array fixes this (instead of expecting the user to add it):

let outputRange = tintColorSecondary.unshift(this.props.tintColor);

This way the tintColor is always used as the first one. Also, you could rename multiColor to something like tintTransitionColors, it might sound more intuitive.

Although, the real solution would be to remove tintColor all together and just have the array color option. I don't know if that'd be too dramatic of a change for old users though.

Neat thought! My idea would be completely removing the secondaryColor and make the tintColor can accept either a single string/array of one string for single element, and array of string for multiple. But I'm not sure how it would affect the library dramatically.

And parse the transition array [0, 50, 100] dynamically unless the user provide the array manually.

For instance, without providing the array and tintColor = {["blue", "red", "green", "orange"]}, the transition array might be something like [0, 33 , 66, 100] . And you can always provide your own array like [0, 25, 50, 100] to adjust the effect but it must follow certain standard in order to use it correctly. There are some edge cases need to be handle also.

[Monir-Shembesh]

Hey @Monir-Shembesh, this looks great. However, appending tintColor to the start of the array fixes this (instead of expecting the user to add it): let outputRange = tintColorSecondary.unshift(this.props.tintColor); This way the tintColor is always used as the first one. Also, you could rename multiColor to something like tintTransitionColors, it might sound more intuitive. Although, the real solution would be to remove tintColor all together and just have the array color option. I don't know if that'd be too dramatic of a change for old users though.

Neat thought! My idea would be completely removing the secondaryColor and make the tintColor can accept either a single string/array of one string for single element, and array of string for multiple. But I'm not sure how it would affect the library dramatically.

And parse the transition array [0, 50, 100] dynamically unless the user provide the array manually.

For instance, without providing the array and tintColor = {["blue", "red", "green", "orange"]}, the transition array might be something like [0, 33 , 66, 100] . And you can always provide your own array like [0, 25, 50, 100] to adjust the effect but it must follow certain standard in order to use it correctly. There are some edge cases need to be handle also.

Hi, i was going to cook some code for this and I actually did but was never able to push it as I got bombarded with work. Check my final solution i came up with to this matter.

animateColor() {

    //for default color value

    if (!this.props.tintTransitionColors) {
      return 'black'
    }

    // to counter the output range issue, we check array length. if it is 1 we return the 0 index of that 
   //array. and now it supports 1 color only if the user wanted.

    if (this.props.tintTransitionColors.length === 1) {
      return this.props.tintTransitionColors[0]
    }

    // here I take and display the colors equally within the wheel. if its 4 colors and the wheel is 100
   // then it will be [0, 25, 50, 75, 100]. if its five colors then it will automatically adapt. 

  

    const colorsArry = this.props.tintTransitionColors.length;
    const inputRange = Array.from({ length: colorsArry }).map(
      (currElement, index) => index * (this.props.circleEndingValue / colorsArry));

//how ever notice that there is a new variable i added called "circleEndingValue" right?
  // well i didnt want my circle to finish at 100, so i change the code structure to accept the value i 
  //want my circle to close at. this is another feature I added which we can send a pull request for 
 //later

    const tintAnimation = this.state.fillAnimation.interpolate({
      inputRange,
      outputRange: this.props.tintTransitionColors
    })

    return tintAnimation

  }

If you guys accept this solution then let me know asap and i will send a pull request and document the README file if any changes are needed. @aboveyunhai @talhaazhar

[talhaazhar]

It looks great. Just make sure to search tintColor in the entire repo and remove it elsewhere!

[aboveyunhai]

Using tintTransitionColors instead of tintColor may make better sense literally if it "intentionally" uses the color transition functionality, but it's primarily used as a "tintColor". And in fact "tintColor " can contain "tintTransitionColors" in literal. I personally disagree with removing and changing tintColor to other names (at least for now unless it's necessary for the functional change in the future).

Most importantly, tintColor is generally set by ppl who uses this library. "Side" change (this PR is nice but a not major change in my opinion) without considering the consequence of will break many apps and cause troubles, unless there was already a solution to make smooth update but I missed out from the discussion.

[Monir-Shembesh]

@aboveyunhai I see your point of view but let me run you through some points.

Documentation is key, tintTransitionColors can contain 1 color or multiple colors thats what we are aiming for here. Not just because the prop name is plural means they must have 2 or more colors.

If you add a literal to tintColor you are making the code more comlicated to the end user/developer who will use this library so KISS (keep it simple stupid) but most importantly functions well as well as being robust. Not to mention that its basically the same point you made where we use a singular noun then pass in multiple colors.

Lastly, this current change will not break on of the apps that are using the current version unless they are directly pulling the repo in their package.json file or they updated to the latest version after the merge. Thus, anyone using the current version will stay safe.

I think the final decision will come down to what the repo owner @bartgryszko sees most suitable. But we are a community and if you feel like you have a better solution please help us make things better :D.

[Monir-Shembesh]

Hi all, please do no merge this branch yet i am going to make change the code to add an end percentage. Will explain more either TMW or After

[fareedagha]

hello #Monir did you change the code that you mentioned above?

[Monir-Shembesh]

@fareedagha sorry for late response. I will be sending the pull request later at night Today.

[fareedagha]

@Monir-Shembesh thanks, please let me know once you done with pull request

[hamol355]

@Monir-Shembesh Is this feature abandoned?

[theyanniss23002]

Guys, we urgently need the ability to multi colors

[markusl]

Guys, we urgently need the ability to multi colors

Would you be able to finish the PR?

[ManigandanRaamanathan]

Is this feature abandoned?

[markusl]

Is this feature abandoned?

@ManigandanRaamanathan if you could finish the PR, fixing the merge issues it can be merged :)