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

[TIMOB-24349]:iOS Ti.UI.AlertDialog Not Firing Events Consistently (run-on-main-thread) #9181

Merged
merged 1 commit into from Jun 30, 2017

Conversation

vijaysingh-axway
Copy link
Contributor

@hansemannn hansemannn self-requested a review June 30, 2017 08:55
@hansemannn
Copy link
Collaborator

CR looks good, but as we discussed internally, we should also look for the underlaying issue that causes issues like this. @vijaysingh-axway will file a ticket for that.

@htbryant94 htbryant94 self-requested a review June 30, 2017 11:34
Copy link

@htbryant94 htbryant94 left a comment

Choose a reason for hiding this comment

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

Approved! using the sample app provided, AlertDialog events fired successfully 100 / 100 times. Tested on both Simulator and Device.

@htbryant94 htbryant94 merged commit 6908e17 into tidev:master Jun 30, 2017
@yustein
Copy link

yustein commented Aug 25, 2017

Gentlemen,

I'm still having this problem with 6.1.2GA. I'm using alert dialogs extensively and experiencing this problem. I will try to compile with 6.1.1GA and see if it improves. If not, I will investigate using native alert dialogs with Hyperloop.

FYI

Kind regards,
Tony

@hansemannn
Copy link
Collaborator

Hey @yustein, thanks for your feedback! The issue is fixed in 6.2.0 (RC next week), you can try it today using appc ti sdk install -b 6_2_X or patch your 6.1.2 with the above changes manually (just a few simple lines of code. Let me know if that helps!

@yustein
Copy link

yustein commented Aug 25, 2017

How ready is 6.2.0 should I install that or make the code change?

Thanks a lot by the way 👍

PS: I have a short but deeply technical Hyperloop question, is it possible if I can ask you that question and if yes should I open an issue somewhere or try to reach you in another way? It is not a bug but it is completely overlooked and thus undocumented IMHO.

@yustein
Copy link

yustein commented Aug 25, 2017

Just let you guys know about a strange thing that happened after installing 6.2.X

I was building the app for ad-hoc testing, I left the computer so it complete the build. After I came back I saw

[INFO] :   Packaging for Ad Hoc distribution
[INFO] :   Deleting dist directory: /Users/xxxxxx/Downloads

I lost more than 40 GB of data in that folder and they are not in my Trash either.

It never deleted that directory before, just put the ipa there.

Deleting explicitly a customer directory without asking permission is considered ... you fill in the dots as if this had happened to you.

I'm still in disbelief.

@yustein
Copy link

yustein commented Aug 25, 2017

And I recreated the same problem with 6.2.X

I lost 40 GB of files for nothing...

@yustein
Copy link

yustein commented Aug 25, 2017

Dear @hansemannn just wanted to alert you, this is serious please read my previous messages

Triple checked it 6.1.2GA does not delete the distribution directory but 6.2.X does.

I can only suggest that someone changes that back, at least put a check and ask user permission when the directory is not empty.

Before what happened to me happens to another person...

@hansemannn
Copy link
Collaborator

hansemannn commented Aug 27, 2017

Hey @yustein, wondering why I didn't see your latest comment earlier, sorry! I am also really sorry for your data loss and hope it was data that can be re-downloaded or restored via Time Machine. This is indeed a serious bug, as we expect to always distribute to some kind of a dedicated dist/ directory in your project directory that is then cleaned-up after building.

I reopened the related ticket (TIMOB-24798) and keep you posted on the updates (feel free to follow that ticket as well). Also please let me know about your Hyperloop-question, you can find me via Slack (@hans), thx!

@yustein
Copy link

yustein commented Aug 27, 2017

@hansemannn no problem, life happens.

One quick question, I was looking at the code for Hyperloop examples for native UIAlertController example and I managed to add a text field but I can't get the content of the textField variable. Can you help me please?

Here is my code (this is important because I can't release my app until the Titanium version of Alert dialog is fixed so I need to use the native version)

(function (container) {

	var UIAlertController = require('UIKit/UIAlertController'),
		UIAlertAction = require('UIKit/UIAlertAction'),
		UIAlertControllerStyleAlert = require('UIKit').UIAlertControllerStyleAlert,
		UIAlertControllerStyleActionSheet = require('UIKit').UIAlertControllerStyleActionSheet,
		UIAlertActionStyleDefault = require('UIKit').UIAlertActionStyleDefault,
		UIAlertActionStyleDestructive = require('UIKit').UIAlertActionStyleDestructive,
		UIAlertActionStyleCancel = require('UIKit').UIAlertActionStyleCancel,
		TiApp = require('Titanium/TiApp');
		
	function showAlertWithStyle(style) {
		
		var textField = "";
		
		var alertController = UIAlertController.alertControllerWithTitleMessagePreferredStyle(
			'My Title',
			'My Message',
			style
		);

		var alertAction = UIAlertAction.actionWithTitleStyleHandler('OK', UIAlertActionStyleDefault, function () {
			$.notice.setText('Clicked OK!');
			console.log(textField);
		});
			
		var cancelAction = UIAlertAction.actionWithTitleStyleHandler('Cancel', UIAlertActionStyleCancel, function () {
			$.notice.setText('Clicked Cancel!');
		});
		
		var destructiveAction = UIAlertAction.actionWithTitleStyleHandler('Delete', UIAlertActionStyleDestructive, function () {
			$.notice.setText('Clicked Delete!');
		});

		alertController.addAction(alertAction);
		alertController.addAction(destructiveAction);
		alertController.addAction(cancelAction);
		alertController.addTextFieldWithConfigurationHandler(textField);
		
		TiApp.app().showModalController(alertController, true);
	}

	$.buttonAlert.addEventListener('click', function () {
		showAlertWithStyle(UIAlertControllerStyleAlert);
	});
	
	$.buttonOptions.addEventListener('click', function () {
		showAlertWithStyle(UIAlertControllerStyleActionSheet);
	});

})($.alert_container);

@hansemannn
Copy link
Collaborator

You are passing a String (textField) into a method (addTextFieldWithConfigurationHandler ) that take a completion-handler as the argument. Here is how your code-snippet should look like (focussing on the configuration-handler):

// The text-field returned in the callback can be configured
alertController.addTextFieldWithConfigurationHandler(function(textField) {
  textField.placeholder = 'Enter value ...';
});

Good luck! And to keep this comments-section clean, try to use Slack if you have these kind of questions, we have an own #hyperloop channel as well! ✌️

@yustein
Copy link

yustein commented Aug 30, 2017

@hansemannn the link you provided https://jira.appcelerator.org/browse/TIMOB-24798 is not the correct issue.

The correct issue ticket is still closed at https://jira.appcelerator.org/browse/TIMOB-24349

I thought adding the native Alert controller would fix this problem but it does not.

AlertController is practically not working at 6.1.2GA period. Unless something changed since the last nightly build 6.2.x also had the problem when I tested.

This is a huge and critical problem, it breaks the UX and Apple might rejects apps with this bug... Even if they don't it is unacceptable from the user's perspective too...

@yustein
Copy link

yustein commented Aug 30, 2017

If you need additional man hours to track and fix this I'm available.

@hansemannn
Copy link
Collaborator

@yustein The ticket is closed again, because it was fixed and QE-tested already yesterday (the adhoc-directory issue).

For your alert-issue, this should be fixed in both 6.2.0 and Hyperloop should work in any way (even without the fix), since it doesn't even touch the SDK. So I highly suspect something in your app is incorrect, intercepting the event in some cases. I had similar issues back in the days when I had events/callbacks intercepting the alert, as the click action triggered the UI before the alert-dialog could complete.

As we did not hear of any related issues since the bugfix, we cannot help you much at this point, unless you write a ticket with a full reproducible test-case, logs and sample-project. I know that sounds much to do and I do understand your frustration, but that's all we can offer you to help and get this out of the way asap.

@yustein
Copy link

yustein commented Aug 30, 2017

screen shot 2017-08-30 at 5 12 26 pm
@hansemannn I don't know the adhoc-directory issue since it has nothing to do about this problem, that is another ticket number...

As you can see from the image attached, that ticket hasn't been updated 30/Jun/17

Why are having this communication problem, this issue is [TIMOB-24349]:iOS Ti.UI.AlertDialog Not Firing Events Consistently (run-on-main-thread) and you are talking about issue number 24798 which is adhoc-directory related, which has nothing to do what we are discussing here...

@hansemannn
Copy link
Collaborator

Hey @yustein, you started the adhoc-discussion above. It was caused by the linked ticket and fixed/closed yesterday. I agree it has nothing to do with the alert-issue, but I wanted to be polite and give you a quick support about it here as well. To not pollute this PR, please either track down the issue in your project or open a JIRA-ticket with the above-provided details, so we can ensure to reproduce it if is an SDK-issue. Thanks.

@yustein
Copy link

yustein commented Aug 30, 2017

Ah you were talking about the 6.2.x deleting my files, now I understand.

FYI that issue's topic is iOS: Cannot Ad Hoc package with Xcode 9 which has nothing to do with 6.2.x deleting my files while building a package. I'm not even using Xcode 9...

I understand that you are being polite and so am I, let's keep it that way please...

Kind regards,
Tony

@hansemannn
Copy link
Collaborator

It was not directly related to Xcode 9, but the parts of the PR that added support for Xcode 9 caused the issue.

@yustein
Copy link

yustein commented Aug 30, 2017

I'm not going to pollute here anymore.

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

4 participants