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

feat(android): add stopAnimation() to View #10130

Closed
wants to merge 39 commits into from
Closed

Conversation

m1ga
Copy link
Contributor

@m1ga m1ga commented Jun 24, 2018

JIRA: https://jira.appcelerator.org/browse/AC-5931

Optional Description:

Android has no possibility to cancel/stop animations like moving an object from left to right before the duration is over. This PR adds a stopAnimation() method to Ti.UI.View

const win = Ti.UI.createWindow();

const lbl = Ti.UI.createLabel({
	text: "-",
	width: Ti.UI.SIZE,
	height: Ti.UI.SIZE
});

const view = Ti.UI.createView({
	backgroundColor: 'red',
	height: 100,
	width: 100,
	top: 0,
	left: 0
});

const btn1 = Ti.UI.createButton({
	title: 'Animate',
	width: 100,
	height: 40,
	bottom: 20,
	left: 10
});
btn1.addEventListener('click', function() {
	view.left = 0;
	view.animate(ani);
});

const btn2 = Ti.UI.createButton({
	title: 'Cancel',
	width: 100,
	height: 40,
	bottom: 20,
	right: 10
});
btn2.addEventListener('click', function() {
	view.stopAnimation();
	view.left = 0;
});

const ani = Ti.UI.createAnimation({
	left: 100,
	duration: 3000
})

ani.addEventListener("start", function() {
	lbl.text = "start";
});

ani.addEventListener("complete", function() {
	lbl.text = "complete";
})

ani.addEventListener("cancel", function() {
	lbl.text = "cancel";
})

win.add([view, btn1, btn2, lbl]);
win.open();

@build
Copy link
Contributor

build commented Jun 24, 2018

Fails
🚫

🔬 There are library changes, but no changes to the unit tests. That's OK as long as you're refactoring existing code, but will require an admin to merge this PR. Please see README.md#unit-tests for docs on unit testing.

🚫

😥 npm test failed. See below for details.

Warnings
⚠️

🔍 Can't find junit reports at ./junit.*.xml, skipping generating JUnit Report.

Messages
📖 🎉 Another contribution from our awesome community member, m1ga! Thanks again for helping us make Titanium SDK better. 👍
📖
> titanium-mobile@8.1.0 test /Users/build/jenkins/workspace/ium-sdk_titanium_mobile_PR-10130
> grunt

Running "appcJs:src:lintOnly" (appcJs) task

Running "eslint:src" (eslint) task

/Users/build/jenkins/workspace/ium-sdk_titanium_mobile_PR-10130/build/packager.js
316:17  warning  Avoid calling back inside of a promise  promise/no-callback-in-promise
317:19  warning  Avoid calling back inside of a promise  promise/no-callback-in-promise

/Users/build/jenkins/workspace/ium-sdk_titanium_mobile_PR-10130/build/scons-xcode-test.js
27:3  warning  Each then() should return a value or throw  promise/always-return

/Users/build/jenkins/workspace/ium-sdk_titanium_mobile_PR-10130/build/utils.js
154:3   warning  Avoid using promises inside of callbacks    promise/no-promise-in-callback
154:3   warning  Avoid using promises inside of callbacks    promise/no-promise-in-callback
154:63  warning  Each then() should return a value or throw  promise/always-return
155:4   warning  Avoid calling back inside of a promise      promise/no-callback-in-promise
157:4   warning  Avoid calling back inside of a promise      promise/no-callback-in-promise
175:59  warning  Each then() should return a value or throw  promise/always-return
176:4   warning  Avoid calling back inside of a promise      promise/no-callback-in-promise
178:4   warning  Avoid calling back inside of a promise      promise/no-callback-in-promise
189:3   warning  Avoid using promises inside of callbacks    promise/no-promise-in-callback
189:3   warning  Avoid using promises inside of callbacks    promise/no-promise-in-callback
189:51  warning  Each then() should return a value or throw  promise/always-return
190:4   warning  Avoid calling back inside of a promise      promise/no-callback-in-promise
192:4   warning  Avoid calling back inside of a promise      promise/no-callback-in-promise
203:71  warning  Each then() should return a value or throw  promise/always-return
205:4   warning  Avoid calling back inside of a promise      promise/no-callback-in-promise
206:17  warning  Avoid calling back inside of a promise      promise/no-callback-in-promise
218:71  warning  Each then() should return a value or throw  promise/always-return
220:4   warning  Avoid calling back inside of a promise      promise/no-callback-in-promise
258:10  warning  Each then() should return a value or throw  promise/always-return
261:18  warning  Avoid calling back inside of a promise      promise/no-callback-in-promise

/Users/build/jenkins/workspace/ium-sdk_titanium_mobile_PR-10130/iphone/cli/commands/_build.js
6364:79  warning  'out' is defined but never used  no-unused-vars

/Users/build/jenkins/workspace/ium-sdk_titanium_mobile_PR-10130/iphone/cli/hooks/frameworks.js
 61:5   warning  Expected catch() or return                  promise/catch-or-return
 61:34  warning  Avoid calling back inside of a promise      promise/no-callback-in-promise
428:5   warning  Each then() should return a value or throw  promise/always-return

✖ 27 problems (0 errors, 27 warnings)


Running "checkFormat:ios" (checkFormat) task

Running "checkFormat:android" (checkFormat) task
Fatal error: Error: Formatting incorrect on "android/titanium/src/java/org/appcelerator/titanium/proxy/TiViewProxy.java", proposed changes: <?xml version='1.0'?>
<replacements xml:space='preserve' incomplete_format='false'>
<replacement offset='21498' length='1'>&#10;						</replacement>
</replacements>
�
npm ERR! Test failed.  See above for more details.

Generated by 🚫 dangerJS against 0766c04

@sgtcoolguy sgtcoolguy self-requested a review June 25, 2018 20:20
Copy link
Contributor

@sgtcoolguy sgtcoolguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @m1ga Thanks for yet another PR!

Some general thoughts:

  • It feels odd to be calling this on the view itself, but I suppose we don't really return an object to call this on otherwise (we don't return something like a Promise to represent the operation). We could potentially call it on a Ti.UI.Animation object that you passed into Ti.UI.View#animate. Makes me curious how this would play with an animation that transitions to a second view...
  • It also doesn't really give you control over exactly which animation it'd try and stop. I suppose only one animation per view at a time is executing given our API.
  • Nor is there any feedback as to whether/when it gets canceled. Seems like for parity there should be a 'canceled' event fired when an animation is canceled. Or maybe at least a boolean return value for this method to indicate that it will attempt to cancel (i.e. if we know there's no animation or it finished already we'd return false)? I don't see a nice backwards-compatible way to modify the complete event to denote an animation was canceled/stopped.

@m1ga
Copy link
Contributor Author

m1ga commented Jun 25, 2018

@sgtcoolguy thanks for your feedback! As far as I remember you can do animate(null) on iOS to stop an animation, so that would be another way to stop the animation on Android. But I have to check that.
Yes, since there is only one animation playing I thought it would be good to stop it on the view. But I'm open for the Ti.UI.Animation way! Just means that you have to keep track of the animation object. In my case I couldn't access it in the 2nd button.
A callback sounds like a good idea. I'll add a cancelled event, so we have start, complete and cancelled to listen for.

@m1ga
Copy link
Contributor Author

m1ga commented Jun 26, 2018

I think I can't make something like animation.stop() since it looks like that there is no connection between TiAnimation (just storing the parameters) and the actual TiAnimationBuilder that is created from the view where the animation is created

@sgtcoolguy
Copy link
Contributor

@m1ga Sorry about the delay in responding!

Ok, yeah it makes sense that We call stopAnimation() on the view then. I'd still like to see some sort of feedback/mechanism like a callback to handle when an animation is canceled.

Also, my OCD hates that both canceled and cancelled are technically correct, and looking at our API docs I do not see that we stuck with one over the other...

@m1ga
Copy link
Contributor Author

m1ga commented Sep 22, 2018

Added the cancel event, updated the example, added the JIRA ticket

@jquick-axway
Copy link
Contributor

jquick-axway commented Nov 20, 2018

@m1ga, calling View.animate() should stop an existing animation too. On iOS, it does. On Android, the previous animation will continue to animate until its duration has ended, which causes the 2nd given animation to be ignored . This is a pre-existing bug in Titanium and not an issue on your end, but this is good opportunity to fix it. Thanks.

Edit:
Actually, let me see if it's possible to do "tweening" between the 2 animations. That would be better than stopping the animation. It's not particularly valuable on Android "yet" since we only support a "linear" animation, but I'd like us to support "easing" style animations on Android in the near future, like iOS.

@m1ga
Copy link
Contributor Author

m1ga commented Nov 21, 2018

@jquick-axway I've added animate() to call stop, too. Should I remove stopAnimation()?

@jquick-axway
Copy link
Contributor

Should I remove stopAnimation()?

Good question... because I'm wondering if setting View.animate() to null would be a good alternative to stopping an animation. Currently, setting it to null will throw an exception on Android, but we can change that behavior. On iOS, a null animation argument is simply ignored, but doesn't stop the current animation (we can change that too).

Side Note:
I've been experimenting with animations this week. Your fix to the View.animate() method solves an edge case where a translation/scale animation needs to change when the app orientation changes. So, thanks for this. :)

@build build added this to the 8.0.0 milestone Dec 20, 2018
@jquick-axway jquick-axway removed this from the 8.0.0 milestone Jan 15, 2019
@sgtcoolguy sgtcoolguy added this to the 9.3.0 milestone Jul 31, 2020
@sgtcoolguy sgtcoolguy modified the milestones: 9.3.0, 10.0.0 Nov 20, 2020
@build build added the docs 📔 label Jan 4, 2022
@hansemannn hansemannn removed this from the 10.0.0 milestone Mar 21, 2022
@hansemannn hansemannn changed the title Android: Add stopAnimation() to View feat(android): add stopAnimation() to View Mar 25, 2022
@m1ga
Copy link
Contributor Author

m1ga commented Mar 27, 2022

great...my master branch is broken after testing the github issue PR 😞 need to fix that
edit: fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants