migrate to AppCompatDialog #3222

agrajaghh · 2015-05-21T17:52:11Z

This migrates from MaterialDialog to AppCompatDialog to eventually eliminate this dependency.
The last other occurence of MaterialDialog is in ReceiveKeyDialog, which I removed in #3225.

I couldn't test the AlertDialog in DeviceProvisioningActivity and in PlayServicesProblemFragment. @rhodey could you help me out there?

agrajaghh · 2015-05-21T17:53:08Z

res/layout/registration_problems.xml

-            android:text="@string/registration_problems__some_possible_problems_include" />
-
-    <TableLayout
+<ScrollView xmlns:android="http://schemas.android.com/apk/res/android"


The diff-view here is not optimal, I just encapsulated everything in a ScrollView:

<ScrollView xmlns:android="http://schemas.android.com/apk/res/android" android:layout_width="fill_parent" android:layout_height="fill_parent" android:paddingTop="24dp" android:paddingLeft="24dp" android:paddingRight="24dp">

mcginty · 2015-05-21T19:42:15Z

protip: you can append ?w=1 to the url to ignore whitespaces changes in a github diff

agrajaghh · 2015-05-21T19:45:21Z

@mcginty WOW! so much better 😳

rhodey · 2015-05-21T20:13:08Z

sure thing @agrajaghh :) I just self assigned and will check back in after I get a chance to test on my N5 & something gingerbread.

rhodey · 2015-05-21T21:28:49Z

@agrajaghh while testing PlayServicesProblemFragment I got this crash.

Caused by: java.lang.IllegalStateException: You need to use a Theme.AppCompat theme (or descendant) with this activity.
            at android.support.v7.app.AppCompatDelegateImplBase.onCreate(AppCompatDelegateImplBase.java:113)
            at android.support.v7.app.AppCompatDelegateImplV7.onCreate(AppCompatDelegateImplV7.java:146)
            at android.support.v7.app.AppCompatDialog.<init>(AppCompatDialog.java:47)
            at android.support.v7.app.AlertDialog.<init>(AlertDialog.java:92)
            at android.support.v7.app.AlertDialog$Builder.create(AlertDialog.java:882)
            at org.thoughtcrime.securesms.PlayServicesProblemFragment.onCreateDialog(PlayServicesProblemFragment.java:38)
            at android.support.v4.app.DialogFragment.getLayoutInflater(DialogFragment.java:308)
            at android.support.v4.app.FragmentManagerImpl.moveToState(FragmentManager.java:955)
            at android.support.v4.app.FragmentManagerImpl.moveToState(FragmentManager.java:1138)
            at android.support.v4.app.BackStackRecord.run(BackStackRecord.java:740)
            at android.support.v4.app.FragmentManagerImpl.execPendingActions(FragmentManager.java:1501)
            at android.support.v4.app.FragmentActivity.onStart(FragmentActivity.java:551)
            at android.app.Instrumentation.callActivityOnStart(Instrumentation.java:1236)
            at android.app.Activity.performStart(Activity.java:6006)
            at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:2288)
            at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:2387)
            at android.app.ActivityThread.access$800(ActivityThread.java:151)
            at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1303)
            at android.os.Handler.dispatchMessage(Handler.java:102)
            at android.os.Looper.loop(Looper.java:135)
            at android.app.ActivityThread.main(ActivityThread.java:5254)
            at java.lang.reflect.Method.invoke(Native Method)
            at java.lang.reflect.Method.invoke(Method.java:372)
            at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:903)
            at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:698)

rhodey · 2015-05-21T21:31:22Z

also for the record all I did to test this was edit NewConversationActivity.onResume() like so

@Override
  public void onResume() {
    super.onResume();
    dynamicTheme.onResume(this);
    dynamicLanguage.onResume(this);
    getSupportActionBar().setTitle(R.string.AndroidManifest__select_contacts);
    startActivity(new Intent(getBaseContext(), PlayServicesProblemActivity.class));
    finish();
  }

and PlayServicesProblemFragment.onCreateDialog() like so

Dialog dialog = /*GooglePlayServicesUtil.getErrorDialog(code, getActivity(), 9111)*/ null;

then just click the new conversation button.

agrajaghh · 2015-05-21T22:29:05Z

@rhodey Thanks! I could have thought of that by myself 😕

I'm using now android:theme="@style/TextSecure.DialogActivity" for PlayServicesProblemActivity and the AlertDialog is working 😊 The Dialog returned by GooglePlayServicesUtil.getErrorDialog() (tested with code=1) is working as well...

Also tested DeviceProvisioningActivity sucessfully.

agrajaghh · 2015-05-26T21:27:09Z

I removed the material-dialogs dependency now from gradle.build since #3225 is in 2.17.0

mcginty · 2015-05-30T01:00:22Z

src/org/thoughtcrime/securesms/DeviceProvisioningActivity.java

+    });
+    final AlertDialog dialog = builder.create();
+    dialog.show();
+    dialog.getButton(AlertDialog.BUTTON_POSITIVE).setOnClickListener(new View.OnClickListener() {


why can't this be done in the empty onClick() handler above?

AppCompatDialog doesn't support .autoDismiss(false), so this is a workaround. Otherwise the dialog gets closed immediately. I just tried to stick to the current behaviour (dialog.dismiss() in onPostExecute()). But maybe thats not necessary, what do you think?

I'd say change handleProvisioning to accept a DialogInterface instead of an AlertDialog and pass along the dialog given to you in the positive button listener.

but how can I prevent the dialog from getting closed immediately? If I do it like that, it will be closed right after the call to handleProvisioning() in the positive button listener and not in onPostExecute() of the ProgressDialogAsyncTask.

What do I miss?

agrajaghh · 2015-06-03T20:37:58Z

rebased

agrajaghh · 2015-06-15T11:17:47Z

rebased and migrated the dialogs from the recent mute/block change

agrajaghh · 2015-06-23T16:08:45Z

@rhodey should I worry about the failed jenkins build?

rhodey · 2015-06-23T16:38:40Z

@agrajaghh nope, looks like the test devices we're using for jenkins hit a rate limit with Google Cloud Messaging registration. after making it through ~20 full tests (~400 GCM registrations) all the tests that required GCM registration started to fail persistently.

jenkins just finished running through the last of the pull requests this morning so I just shut that down temporarily. I intend to let it idle for a few hours, maybe a day, and then turn it back on and re-run all of the PRs it marked as failed while somehow trying to avoid the GCM rate limit again.

I don't expect this GCM rate limit will be a problem once we get over the initial backlog, sorry for the trouble.

agrajaghh · 2015-06-23T16:56:30Z

no trouble, just curious, thanks for the explanation ☺️

agrajaghh · 2015-08-02T20:51:44Z

rebased, migrated the dialogs in DeviceListActivity and had to remove
<item name="android:backgroundDimEnabled">false</item> (introduced in 2016fa3) because there isn't a shadow on GB around the dialogs which makes them indistinguishable from the normal activity. If this is a problem I could just disable android:backgroundDimEnabled for >= Honeycomb?

agrajaghh · 2015-08-05T21:10:14Z

since #3883 is in 2.25.0 now, I reverted the last commit because its not needed anymore without android:theme="@style/TextSecure.LightTheme.Popup" for ConversationActivity

Successfully tested on top of 2.25.0 (a3a7f8f) on 5.1.1 and 2.3.7

This reverts commit 0fbbe399e3fa01c1744321e934d4e4d7a04eae81.

agrajaghh · 2015-08-06T19:16:09Z

rebased again

should I continue/is this on the nearby agenda? 😟

mcginty · 2015-08-06T20:33:11Z

src/org/thoughtcrime/securesms/DeviceProvisioningActivity.java

-                     })
-                     .show();
+    AlertDialog.Builder builder = new AlertDialog.Builder(this);
+    builder.setTitle(R.string.DeviceProvisioningActivity_link_this_device);


Why not chain these method calls like before?

AlertDialog dialog = new AlertDialog.builder(this).setTitle(R.blah.blahdeblah) .setPositiveBlahblah() .create();

mcginty · 2015-08-06T21:38:37Z

src/org/thoughtcrime/securesms/DeviceProvisioningActivity.java

+            .setMessage(content)
+            .setPositiveButton(R.string.DeviceProvisioningActivity_continue, new DialogInterface.OnClickListener() {
+              @Override
+              public void onClick(DialogInterface dialog, int which) {


I think it'd make more sense to have handleProvisioning take in a DialogInterface instead of an AlertDialog and then you can use the normal style of setting OnClickListeners.

I feel stupid now 😕 I tried that, see my comment above:

but how can I prevent the dialog from getting closed immediately? If I do it like that, it will be closed right after the call to handleProvisioning() in the positive button listener and not in onPostExecute() of the ProgressDialogAsyncTask.

What do I miss?

ah sorry, my comment got hidden so I didn't think I posted it for some reason!

I see what you're saying now, yeah we have to either do it this way or implement a custom non-alert dialog. The latter is probably less hacky but more effort.

puh, I thought I missed something obvious 😌

Thats your call, I think these +7 lines are okay and way less than a custom dialog ;)

mcginty · 2015-11-17T20:56:59Z

rebased, squashed, migrated RedPhone dialogs, sanity checked (including the device provisioning and play services problems dialogs). time to merge :)

in 3.5.0

closes #3222

agrajaghh · 2015-11-17T21:37:11Z

wow, I almost lost hope 😝

closes #3222

Closes signalapp/Signal-Android#3222 Fixes #291 Upstream commit: signalapp/Signal-Android@c433981

agrajaghh reviewed May 21, 2015
View reviewed changes

agrajaghh mentioned this pull request May 21, 2015

use ConfirmIdentityDialog instead of ReceiveKeyDialog #3225

Closed

rhodey self-assigned this May 21, 2015

mcginty reviewed May 30, 2015
View reviewed changes

agrajaghh force-pushed the AppCompatDialog branch from 570a15b to da16b89 Compare June 3, 2015 20:37

agrajaghh force-pushed the AppCompatDialog branch from da16b89 to 6ecc1b5 Compare June 15, 2015 11:16

agrajaghh force-pushed the AppCompatDialog branch from 6ecc1b5 to 11ff243 Compare June 23, 2015 08:53

moxie0 assigned mcginty and unassigned rhodey Jun 24, 2015

moxie0 force-pushed the master branch from 1564049 to 0a3b78f Compare July 24, 2015 22:14

agrajaghh force-pushed the AppCompatDialog branch from 11ff243 to 28f805b Compare August 2, 2015 20:46

agrajaghh force-pushed the AppCompatDialog branch from 28f805b to c077825 Compare August 5, 2015 21:10

agrajaghh added 4 commits August 6, 2015 21:05

migrate to AppCompatDialog

6f05e0b

fix for PlayServicesProblemFragment

57f79e3

remove material-dialogs dependency

67b5194

migrate dialogs from the mute/block change

255e539

agrajaghh added 3 commits August 6, 2015 21:05

replace dialogs in DeviceListActivity

17146cc

remove android:backgroundDimEnabled=false

3d7efa6

Revert "remove android:backgroundDimEnabled=false"

ce7883e

This reverts commit 0fbbe399e3fa01c1744321e934d4e4d7a04eae81.

agrajaghh force-pushed the AppCompatDialog branch from c077825 to ce7883e Compare August 6, 2015 19:16

mcginty reviewed Aug 6, 2015
View reviewed changes

chain AlertDialog.Builder methods

29dfc11

mcginty reviewed Aug 6, 2015
View reviewed changes

agrajaghh mentioned this pull request Sep 29, 2015

Upgrade material-dialogs #4138

Closed

agrajaghh mentioned this pull request Nov 16, 2015

Fix gradle (material-dialogs) - fixes #4138 #4562

Closed

mcginty closed this Nov 17, 2015

mcginty pushed a commit that referenced this pull request Nov 17, 2015

migrate to AppCompatDialog

5e00869

closes #3222

agrajaghh deleted the AppCompatDialog branch November 17, 2015 21:37

moxie0 pushed a commit that referenced this pull request Nov 19, 2015

migrate to AppCompatDialog

c433981

closes #3222

BLeQuerrec pushed a commit to SilenceIM/Silence that referenced this pull request Dec 9, 2015

migrate to AppCompatDialog

e5aa561

Closes signalapp/Signal-Android#3222 Fixes #291 Upstream commit: signalapp/Signal-Android@c433981

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

migrate to AppCompatDialog #3222

migrate to AppCompatDialog #3222

agrajaghh commented May 21, 2015

agrajaghh May 21, 2015

mcginty commented May 21, 2015

agrajaghh commented May 21, 2015

rhodey commented May 21, 2015

rhodey commented May 21, 2015

rhodey commented May 21, 2015

agrajaghh commented May 21, 2015

agrajaghh commented May 26, 2015

mcginty May 30, 2015

agrajaghh May 30, 2015

mcginty Aug 6, 2015

agrajaghh Aug 6, 2015

agrajaghh commented Jun 3, 2015

agrajaghh commented Jun 15, 2015

agrajaghh commented Jun 23, 2015

rhodey commented Jun 23, 2015

agrajaghh commented Jun 23, 2015

agrajaghh commented Aug 2, 2015

agrajaghh commented Aug 5, 2015

agrajaghh commented Aug 6, 2015

mcginty Aug 6, 2015

agrajaghh Aug 6, 2015

mcginty Aug 6, 2015

agrajaghh Aug 6, 2015

mcginty Aug 6, 2015

agrajaghh Aug 6, 2015

mcginty commented Nov 17, 2015

agrajaghh commented Nov 17, 2015

migrate to AppCompatDialog #3222

migrate to AppCompatDialog #3222

Conversation

agrajaghh commented May 21, 2015

Choose a reason for hiding this comment

mcginty commented May 21, 2015

agrajaghh commented May 21, 2015

rhodey commented May 21, 2015

rhodey commented May 21, 2015

rhodey commented May 21, 2015

agrajaghh commented May 21, 2015

agrajaghh commented May 26, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agrajaghh commented Jun 3, 2015

agrajaghh commented Jun 15, 2015

agrajaghh commented Jun 23, 2015

rhodey commented Jun 23, 2015

agrajaghh commented Jun 23, 2015

agrajaghh commented Aug 2, 2015

agrajaghh commented Aug 5, 2015

agrajaghh commented Aug 6, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcginty commented Nov 17, 2015

agrajaghh commented Nov 17, 2015