refactor(android): ProgressIndicator dialog handling #11115

jquick-axway · 2019-08-06T05:26:30Z

JIRA:

Summary:

TIMOB-27104 Fixed bug where ProgressIndicator logs "WindowLeaked" exception when hiding dialog and closing window at same time as of 8.0.2.
TIMOB-27308 Fixed bug where determinant ProgressIndicator ignores "value" property before shown.
TIMOB-27309 Fixed bug where you cannot re-show ProgressIndicator dialog if auto-closed by previous window.
Removed dialog's handleMessage() related code. No longer needed since JS runs on main UI thread.

Note:
The status bar ProgressIndicator is a dead feature. Google no longer supports it and their Java APIs effectively no-op. I've written up ticket TIMOB-27312 to refactor it in the future. In the meantime, there is no point in testing it. (I've already done some testing on it anyways with this PR and it's no worse.)

Test 1:
(Verifies dialog no longer logs exception when window closes.)

Build and run the code attached to TIMOB-27104 on Android.
Tap on the "Show Progress Dialog" button.
Wait 2 seconds for the progress dialog and its window to close.
Verify that a "WindowLeaked" exception was NOT logged.

Test 2:
(Verifies progress indicator reads "value" property when shown.)

Build and run the code attached to TIMOB-27308 on Android.
Tap on the "Show Progress Dialog" button.
Verify that the dialog shows 50%.

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

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

Test 4:
(Verifies determinant progress indicator handling.)

Build and run with the below code on Android.
Tap on the "Show Progress Dialog" button.
Verify that a progress dialog is shown.
Verify that its progress bar slowly fills from 0% to 100% and then closes itself.
Tap on the "Show Progress Dialog" button.
Tap on the Android "back" button to close the progress dialog.
Tap on the "Show Progress Dialog" button.
Verify dialog is shown again and starts at 0%.
Verify dialog auto-closes itself when it reaches 100%.

var timerId = null;
var progressIndicator = Ti.UI.Android.createProgressIndicator({
	message: "Progressing...",
	location: Ti.UI.Android.PROGRESS_INDICATOR_DIALOG,
	type: Ti.UI.Android.PROGRESS_INDICATOR_DETERMINANT,
	cancelable: true,
	min: 0,
	max: 100,
});
progressIndicator.addEventListener("cancel", function() {
	if (timerId) {
		clearInterval(timerId);
		timerId = null;
	}
});

var window = Ti.UI.createWindow();
var showButton = Ti.UI.createButton({
	title: "Show Progress Dialog",
	bottom: "30dp",
});
showButton.addEventListener("click", function() {
	progressIndicator.value = 0;
	progressIndicator.show();
	timerId = setInterval(function() {
		var value = progressIndicator.value;
		if (value >= progressIndicator.max) {
			progressIndicator.hide();
			clearInterval(timerId);
			timerId = null;
		} else {
			if (!value) {
				value = progressIndicator.min;
			}
			value += (progressIndicator.max - progressIndicator.min) / 10;
			progressIndicator.value = value;
		}
	}, 250);
});
window.add(showButton);
window.open();

- [TIMOB-27104] ProgressIndicator logs "WindowLeaked" exception when hiding dialog and closing window at same time as of 8.0.2 - [TIMOB-27308] Determinant ProgressIndicator ignores "value" property before shown - [TIMOB-27309] Cannot re-show ProgressIndicator dialog if auto-closed by previous window - Removed dialog's handleMessage() related code. No longer needed since JS runs on main UI thread.

build · 2019-08-06T06:06:11Z

	Fails
🚫	Tests have failed, see below for more information.

	Messages
📖	💾 Here's the generated SDK zipfile.
📖	❌ 2 tests have failed There are 2 tests failing and 472 skipped out of 4354 total tests.
📖	✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.

Tests:

Classname

Name

Time

Error

ios.Titanium.UI.iOS

#createPreviewContext()

0.002

Error: expected 0 to equal 300
fail@file:///Users/build/Library/Developer/CoreSimulator/Devices/8161F144-7CF2-427F-9592-E4B4EA7537FF/data/Containers/Bundle/Application/5AC7B4FD-398F-42FE-9831-DDE3BA4405DF/mocha.app/node_modules/should/cjs/should.js:275:23
value@file:///Users/build/Library/Developer/CoreSimulator/Devices/8161F144-7CF2-427F-9592-E4B4EA7537FF/data/Containers/Bundle/Application/5AC7B4FD-398F-42FE-9831-DDE3BA4405DF/mocha.app/node_modules/should/cjs/should.js:356:23
file:///Users/build/Library/Developer/CoreSimulator/Devices/8161F144-7CF2-427F-9592-E4B4EA7537FF/data/Containers/Bundle/Application/5AC7B4FD-398F-42FE-9831-DDE3BA4405DF/mocha.app/ti.ui.ios.previewcontext.test.js:26:48
file:///Users/build/Library/Developer/CoreSimulator/Devices/8161F144-7CF2-427F-9592-E4B4EA7537FF/data/Containers/Bundle/Application/5AC7B4FD-398F-42FE-9831-DDE3BA4405DF/mocha.app/ti-mocha.js:4376:41
file:///Users/build/Library/Developer/CoreSimulator/Devices/8161F144-7CF2-427F-9592-E4B4EA7537FF/data/Containers/Bundle/Application/5AC7B4FD-398F-42FE-9831-DDE3BA4405DF/mocha.app/ti-mocha.js:4763:17
file:///Users/build/Library/Developer/CoreSimulator/Devices/8161F144-7CF2-427F-9592-E4B4EA7537FF/data/Containers/Bundle/Application/5AC7B4FD-398F-42FE-9831-DDE3BA4405DF/mocha.app/ti-mocha.js:4840:23
next@file:///Users/build/Library/Developer/CoreSimulator/Devices/8161F144-7CF2-427F-9592-E4B4EA7537FF/data/Containers/Bundle/Application/5AC7B4FD-398F-42FE-9831-DDE3BA4405DF/mocha.app/ti-mocha.js:4688:20
file:///Users/build/Library/Developer/CoreSimulator/Devices/8161F144-7CF2-427F-9592-E4B4EA7537FF/data/Containers/Bundle/Application/5AC7B4FD-398F-42FE-9831-DDE3BA4405DF/mocha.app/ti-mocha.js:4698:15
next@file:///Users/build/Library/Developer/CoreSimulator/Devices/8161F144-7CF2-427F-9592-E4B4EA7537FF/data/Containers/Bundle/Application/5AC7B4FD-398F-42FE-9831-DDE3BA4405DF/mocha.app/ti-mocha.js:4636:30
file:///Users/build/Library/Developer/CoreSimulator/Devices/8161F144-7CF2-427F-9592-E4B4EA7537FF/data/Containers/Bundle/Application/5AC7B4FD-398F-42FE-9831-DDE3BA4405DF/mocha.app/ti-mocha.js:4665:13
timeslice@file:///Users/build/Library/Developer/CoreSimulator/Devices/8161F144-7CF2-427F-9592-E4B4EA7537FF/data/Containers/Bundle/Application/5AC7B4FD-398F-42FE-9831-DDE3BA4405DF/mocha.app/ti-mocha.js:5764:29

ios.Titanium.UI.Window

.safeAreaPadding for window inside navigation window with extendSafeArea true

0.006

Error: expected 34 to equal 0
fail@file:///Users/build/Library/Developer/CoreSimulator/Devices/8161F144-7CF2-427F-9592-E4B4EA7537FF/data/Containers/Bundle/Application/5AC7B4FD-398F-42FE-9831-DDE3BA4405DF/mocha.app/node_modules/should/cjs/should.js:275:23
value@file:///Users/build/Library/Developer/CoreSimulator/Devices/8161F144-7CF2-427F-9592-E4B4EA7537FF/data/Containers/Bundle/Application/5AC7B4FD-398F-42FE-9831-DDE3BA4405DF/mocha.app/node_modules/should/cjs/should.js:356:23
listener@file:///Users/build/Library/Developer/CoreSimulator/Devices/8161F144-7CF2-427F-9592-E4B4EA7537FF/data/Containers/Bundle/Application/5AC7B4FD-398F-42FE-9831-DDE3BA4405DF/mocha.app/ti.ui.window.test.js:721:38

Generated by 🚫 dangerJS against 5ac474c

build · 2019-08-07T04:03:45Z

tests/Resources/ti.ui.android.progressindicator.addontest.js

+ * Please see the LICENSE included with this distribution for details.
+ */
+/* eslint-env mocha */
+/* global Titanium */


🚫 tests/Resources/ti.ui.android.progressindicator.addontest.js line 8 – 'Titanium' is already defined as a built-in global variable. (no-redeclare)

⚠️ tests/Resources/ti.ui.android.progressindicator.addontest.js line 8 – 'Titanium' is defined but never used. (no-unused-vars)

garymathews

CR: PASS

ssekhri · 2019-08-20T22:10:33Z

FR Passed. The progress indicator works as expected.
The value set before the show() is shown on the progress indicator. Also the progress dialog is successfully re-shown and no error is shown in logs.
Verified on:
Mac OS 10.14.6
Ti SDK: 8.2.0.v20190820142426
Appc CLI: 7.1.1-master.2
Node: 10.5.0
JDK: 10.0.2
Studio: 5.1.3.201907112159

jquick-axway added android bug work in progress 🚧 needs tests 🚨 labels Aug 6, 2019

jquick-axway added this to the 8.2.0 milestone Aug 6, 2019

jquick-axway requested a review from garymathews August 6, 2019 05:26

build requested a review from a team August 6, 2019 06:06

chore(android): added unit test for Ti.UI.Android.ProgressIndicator

41894f9

jquick-axway removed needs tests 🚨 work in progress 🚧 labels Aug 7, 2019

build reviewed Aug 7, 2019

View reviewed changes

This was referenced Aug 13, 2019

refactor(android)(8_1_X): ProgressIndicator dialog handling #11142

Merged

refactor(android)(8_3_X): ProgressIndicator dialog handling #11143

Merged

jquick-axway and others added 2 commits August 13, 2019 18:11

refactor(android): update [TIMOB-27104] based on feedback

3087e09

Merge branch 'master' into TIMOB-27104

9efafa2

garymathews approved these changes Aug 20, 2019

View reviewed changes

Merge branch 'master' into TIMOB-27104

472fb32

build added the in-qe-testing 🕵 label Aug 20, 2019

Merge branch 'master' into TIMOB-27104

5ac474c

sgtcoolguy merged commit 6a656b9 into tidev:master Aug 22, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(android): ProgressIndicator dialog handling #11115

refactor(android): ProgressIndicator dialog handling #11115

jquick-axway commented Aug 6, 2019 •

edited

build commented Aug 6, 2019 •

edited

build Aug 7, 2019

garymathews left a comment

ssekhri commented Aug 20, 2019

refactor(android): ProgressIndicator dialog handling #11115

refactor(android): ProgressIndicator dialog handling #11115

Conversation

jquick-axway commented Aug 6, 2019 • edited

build commented Aug 6, 2019 • edited

Tests:

build Aug 7, 2019

Choose a reason for hiding this comment

garymathews left a comment

Choose a reason for hiding this comment

ssekhri commented Aug 20, 2019

jquick-axway commented Aug 6, 2019 •

edited

build commented Aug 6, 2019 •

edited