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(android): hide/show progress dialog back-to-back puts it in bad state as if 8.1.1 #11684

Merged
merged 3 commits into from May 12, 2020

Conversation

jquick-axway
Copy link
Contributor

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

Summary:
Calling progress dialog's hide() and then immediately show() back-to-back prevents the dialog from ever being hidden again. Caused by the Java onDismiss() listener for previous dialog being invoked just after show() method call for the new dialog.

Test 1:
(Verifies the above ticket's fix.)

  1. Build and run the code attached to TIMOB-27573 on Android.
  2. Tap on the "Show Progress Dialog" button.
  3. Verify that dialog is shown.
  4. Verify that dialog is quickly hidden/shown 3 seconds later.
  5. Verify that dialog is hidden 2 seconds. (This used to fail.)

Test 2:
(Verifies progress dialog can be re-shown after auto-closed.)

  1. Build and run the code attached to TIMOB-27309 on Android.
  2. Tap on the "Show Progress Dialog" button.
  3. Wait 2 seconds for the progress dialog and its window to close.
  4. Tap on the "Show Progress Dialog" button again.
  5. Verify that the progress dialog is re-shown.

…tate

- Regression was introduced as of Titanium 8.1.1.
@jquick-axway jquick-axway added this to the 9.1.0 milestone May 5, 2020
@jquick-axway jquick-axway changed the title fix(android): hide->show progress dialog back-to-back puts it in bad state as if 8.1.1 fix(android): hide/show progress dialog back-to-back puts it in bad state as if 8.1.1 May 5, 2020
@build build requested a review from a team May 5, 2020 03:32
@build
Copy link
Contributor

build commented May 5, 2020

Warnings
⚠️

Commit 9a131b4488521b6033a151ac326cda02af386688 has a message "fix(android): hide/show progress dialog back-to-back puts it in bad state

  • Regression was introduced as of Titanium 8.1.1." giving 1 errors:
  • header must not be longer than 72 characters, current length is 73
Messages
📖

💾 Here's the generated SDK zipfile.

📖

🚨 This PR has one or more commits with warnings/errors for commit messages not matching our configuration. You may want to squash merge this PR and edit the message to match our conventions, or ask the original developer to modify their history.

📖

✅ All tests are passing
Nice one! All 6574 tests are passing.
(There are 708 skipped tests not included in that total)

Generated by 🚫 dangerJS against 8720514

@hansemannn
Copy link
Collaborator

Can this be a 9.0.2 candidate as it seems to be a regression? We may run into this as well.

@jquick-axway
Copy link
Contributor Author

jquick-axway commented May 5, 2020

@hansemannn, the team doesn't want to add anymore tickets to 9.0.2 unless it's a show-stopper issue, because we plan on doing a release soon.

But you shouldn't be running into this issue unless you do exactly this...

const window = Ti.UI.createWindow();
const progressIndicator = Ti.UI.Android.createProgressIndicator({
	message: "Loading...",
	location: Ti.UI.Android.PROGRESS_INDICATOR_DIALOG,
	cancelable: true,
});
window.addEventListener("open", () => {
	progressIndicator.show();
	setTimeout(() => {
		// Calling hide/show back-to-back causes next hide() to not work.
		progressIndicator.hide();
		progressIndicator.show();
	}, 1000);
});
window.open();

@hansemannn
Copy link
Collaborator

Yep, thats exactly what we do - showing concurring loaders. But with different instances. Does it not happen then? Then we're fine!

Copy link
Contributor

@garymathews garymathews left a comment

Choose a reason for hiding this comment

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

CR: PASS

@ssekhri
Copy link

ssekhri commented May 12, 2020

FR Passed.

Verified on:
Mac OS: 10.15.4
SDK: 9.1.0.v20200511102344
Appc CLI: 8.0.0
JDK: 11.0.4
Node: 10.16.3
Studio: 6.0.0.202005071607
Device: Nexus4(v5.1.1) device, Pixel3(v10.0) emulator

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

5 participants