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

fix(ios): animating view width/height from zero #12832

Merged
merged 3 commits into from Jun 14, 2021

Conversation

jquick-axway
Copy link
Contributor

@jquick-axway jquick-axway commented May 21, 2021

JIRA:
https://jira.appcelerator.org/browse/TIMOB-27236

Summary:

  • On iOS, animating a view's width/height doesn't work if current width or height is zero. The animation "complete" event will never fire either.
  • Not an issue on Android.

Test:

  1. Build and run AnimationSizeTest.js attached to TIMOB-27236.
  2. Notice the window is white.
  3. Tap on the "Expand/Collapse" button.
  4. Verify an orange view expands from top/left corner and fills the window. (This is the fix.)
  5. Tap on the "Expand/Collapse" button again.
  6. Verify the orange view collapses to the top/left corner and disappears.

@jquick-axway jquick-axway added ios bug backport 10_2_X when applied, PRs with this label will get an auto-generated backport to 10_2_X branch on merge labels May 21, 2021
@jquick-axway jquick-axway added this to the 10.1.0 milestone May 21, 2021
@build build requested a review from a team May 21, 2021 22:20
@build
Copy link
Contributor

build commented May 21, 2021

Fails
🚫 Tests have failed, see below for more information.
Messages
📖 ✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖 ❌ 3 tests have failed There are 3 tests failing and 961 skipped out of 15378 total tests.
📖

💾 Here's the generated SDK zipfile.

Tests:

ClassnameNameTimeError
android.emulator.Titanium.UI.ListView.refreshControl (in NavigationWindow) (5.0.2)10.452
Error: Timeout of 5000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (app.js)
android.emulator.Titanium.UI.View.backgroundImage (URL-redirect) (5.0.2)10.382
Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (app.js)
ios.macos.Titanium.UI.Viewanimate width/height from zero (10.15.5)10.082
Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (app.js)

Generated by 🚫 dangerJS against 3776249

Copy link
Contributor

@janvennemann janvennemann left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -1318,8 +1318,7 @@ - (BOOL)viewInitialized

- (BOOL)viewReady
{
return view != nil && !CGRectIsEmpty(view.bounds) && !CGRectIsNull(view.bounds) &&
[view superview] != nil;
return (view != nil) && ([view superview] != nil);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is just me being super nitpicky, but the brackets are not really required here.

@lokeshchdhry
Copy link
Contributor

FR Passed.

@sgtcoolguy sgtcoolguy merged commit df7c106 into tidev:master Jun 14, 2021
@build build removed the backport 10_2_X when applied, PRs with this label will get an auto-generated backport to 10_2_X branch on merge label Jun 14, 2021
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.

None yet

6 participants