search

feathersui / feathersui-starling

User interface components for Starling Framework and Adobe AIR
https://feathersui.com/learn/as3-starling/
Other
914 stars 386 forks source link

FeathersControl.visible now checks corresponding effect context #1789

Closed esidegallery closed 5 years ago

esidegallery commented 5 years ago

I noticed that the commit fixing issue #1787 won't always work as expected. For example, hideEffect only sets super.visible to false when it's finished running. If hideEffect is running, setting visible = true will be ignored allowing the effect to continue, and setting to false will interrupt the effect. I have updated it so that it returns only if the new value's corresponding effect is running. Otherwise it carries on as before. I haven't fully tested this, as I'm running the SDK.

joshtynjala commented 5 years ago

I'd like for it to return early consistently, even if there is no showEffect or hideEffect specified. Technically, it shouldn't matter if you return early or not when there are no effects, but I prefer not having to take that extra few seconds to think about it when I'm reading through the code.

With that in mind, I think that the new value of visible probably needs to be stored in a variable so that it can be checked before the effect completes. Can you make that change?

esidegallery commented 5 years ago

Something like this? The new value is stored in a pending variable, which is applied when appropriate.

joshtynjala commented 5 years ago

Looks good to me. I'll merge it once I see that Travis CI has finished running tests.

esidegallery commented 5 years ago

Great, thanks Josh.

esidegallery commented 5 years ago

p.s. I missed a this on line 3698!

joshtynjala commented 5 years ago

p.s. I missed a this on line 3698!

Yeah, I noticed. 😄

I can fix it after the merge.