[animation-timeline-component] Loop property value bug

[jasr88]

Bug:

• When you try to modify the loop property value via script, if you set the value to true (boolean type), the result is loop = NaN
  Expected Behaviour:
• When you try to modify the loop property value via script, if you set the value to true (boolean type), the result is loop = true.

Solution:

• Add a validation for the property loop asking if the value provides is already boolean, if it is, then return it without furter actions.

[vincentfretin]

If you want your PR to be merged, you should modify the animation-timeline/index.js file, not the file in the dist folder. Be careful to use the same indentation too.

[jasr88]

Hi, thanks for the response I'll be careful about formatting and indentation next time.

The problem is that now that I've checked the animation-timeline/index.js the bug is not there:

//This is correct!
      parse: function (value) {
        // Boolean or integer.
        if (value === true || value === 'true') { return true; }
        if (value === false || value === 'false') { return false; }
        return parseInt(value, 10);
      }

But in the dist folder, the problem persists. Now I know that this pull request has no sense, but, maybe can I ask to update the dis/animation-timeline-component.js code?

For now, we are using a workaround converting the boolean value to a string, I just want to save some time for those how have faced this problem.

[vincentfretin]

Hi, just to be clear I'm not a maintainer of the repository, and I don't use the component myself, so I'm not the one that can change anything here. :-)
Usually the files in dist are created by the npm run build or npm run dist command as part of the release process by the package maintainer. Here you can run in the animation-timeline folder:

npm install
npm run dist

to update the dist/aframe-animation-timeline-component.min.js and dist/aframe-animation-timeline-component.js files for yourself, but don't include them in the PR for an easier review by the maintainer. The maintainer will take care of updating the files once the PR is merged.