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

Deprecate usage of begin as a delay and provide a delay attribute, instead. #1508

Merged
merged 17 commits into from May 27, 2016

Conversation

@msimpson
Copy link
Contributor

@msimpson msimpson commented May 27, 2016

Start to address the concerns raised in #1401.

Matt Simpson added 16 commits May 12, 2016
// Deprecation warning for begin when used as a delay.
if (data.begin !== '' && !isNaN(data.begin)) {
console.warn("Using 'begin' to specify a delay is deprecated. Use 'delay' instead.");
data.delay = cloneValue(data.begin);

This comment has been minimized.

@ngokevin

ngokevin May 27, 2016
Member

Don't think you need to clone because it's just a number.

@@ -161,7 +161,7 @@ module.exports.AAnimation = registerElement('a-animation', {
// Deprecation warning for begin when used as a delay.
if (data.begin !== '' && !isNaN(data.begin)) {
console.warn("Using 'begin' to specify a delay is deprecated. Use 'delay' instead.");
data.delay = cloneValue(data.begin);
data.delay = data.begin;

This comment has been minimized.

@msimpson

msimpson May 27, 2016
Author Contributor

Gotcha.

@@ -114,7 +114,7 @@ module.exports.AAnimation = registerElement('a-animation', {
self.count = repeat === Infinity ? 0 : parseInt(data.repeat, 10);
}

if (isNaN(begin)) { begin = 0; }
if (isNaN(delay)) { delay = 0; }

This comment has been minimized.

@msimpson

msimpson May 27, 2016
Author Contributor

Actually, wouldn't it be safe to remove this check as well?

This comment has been minimized.

@ngokevin

ngokevin May 27, 2016
Member

Maybe good to leave in to handle weird input

This comment has been minimized.

@msimpson

msimpson May 27, 2016
Author Contributor

Okay.

@ngokevin ngokevin merged commit f0e6e09 into aframevr:master May 27, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants