fix(pauseResume): Fix a script error on stop() #276
Conversation
When you call it on elements which is not animated, script error occurs because __aniProp is not exist. Ref naver#274
|
LGTM |
|
LGTM 2 |
| @@ -10,13 +10,46 @@ QUnit.config.reorder = false; | |||
| var hooks = { | |||
| beforeEach: function() { | |||
| this.$el = $("#box1"); | |||
| this.$el.each(function() { | |||
There was a problem hiding this comment.
it can be silly question, but $el is referring an element which has id. Assuming that, $el.each doesn't make sense, considering $el is an element not array.
There was a problem hiding this comment.
@netil Yes, you right!
But as you know, we should handle element as array item no matter how many it has.
So in any case we need to access that element using array style.
But I didn't want to access an element by [0] like below. It looks hard-corded.
delete $el[0].__aniProps;There was a problem hiding this comment.
The way of handling will dependent of data types. So if we're handling array is ok using array methods, but if we handle non array values is quite considerable use non array method for that.
In my opinion for that case I'd write like:
this.$el = $("#box1");
delete this.$el.get(0).__aniProps;
// but, also below one is ok
delete $el[0].__aniProps;But, it's not a big deal issue. It's ok what is written now.
There was a problem hiding this comment.
@netil I agree with you in this context. Thank you for review!
skip: apply review
Issue
#274
Details
When you call stop() on elements which is not animated,
script error occurs because __aniProp is not exist.
Preferred reviewers
@naver/egjs-dev