App installation screen improvements #680

rosenpin · 2017-05-08T15:24:22Z

Improvements include:

Users can now choose multiple files to install at once
Progress bar and an appropriate message when an app is successfully/unsuccessfully installed
Replaced RelativeLayout with LinearLayout where it seems to be more appropriate (activity_appinstaller.xml)

…nstallation activity

danielegobbetti · 2017-05-08T16:16:00Z

This is really interesting, thanks!

Especially wrt the layout changes, this should be carefully tested also with firmware installation and for other devices as well.

rosenpin · 2017-05-08T18:01:34Z

app/src/main/AndroidManifest.xml

@@ -65,8 +65,7 @@
            android:parentActivityName=".activities.SettingsActivity" />
        <activity
            android:name=".activities.FwAppInstallerActivity"
-            android:label="@string/title_activity_fw_app_insaller"
-            android:parentActivityName=".activities.ControlCenterv2">


Removed the parent activity because we don't want the back button to re-open the main activity, we want it to finish the current activity

rosenpin · 2017-05-08T18:02:24Z

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/FwAppInstallerActivity.java

@@ -128,6 +148,12 @@ private void validateInstallation() {
    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
+        if (GBApplication.isDarkThemeEnabled()) {


the super.onCreate sets the theme to GadgetbridgeThemeDark with action bar, we don't want the action bar because we use a toolbar in the layout xml file

Could you try directly extending the appcompat activity as it is done in ControlCenterv2? https://github.com/Freeyourgadget/Gadgetbridge/blob/master/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/ControlCenterv2.java#L63
It should allow you to avoid this check.

Done, I also merged the toolbars by using a toolbar.xml file, I include that file both in the ControlCenterv2 activity and the FwAppInstallerActivity activity layouts

I still need to check if the dark theme is enabled and change the theme if it is. But the else statement is redundant

rosenpin · 2017-05-08T18:03:04Z

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/FwAppInstallerActivity.java

+                IntentFilter intentFilter = new IntentFilter();
+                intentFilter.addAction("nodomain.freeyourgadget.gadgetbridge.INSTALLATION_FAILED");
+                intentFilter.addAction("nodomain.freeyourgadget.gadgetbridge.INSTALLATION_SUCCESS");
+                registerReceiver(installationResultReceiver, intentFilter);


Register the receiver to the failed/success installation intent, the receiver updates the views (progress bar/button)

rosenpin · 2017-05-08T18:04:04Z

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/FwAppInstallerActivity.java

+
+        //Set up the toolbar
+        Toolbar toolbar = (Toolbar) findViewById(R.id.toolbar);
+        toolbar.setTitleTextColor(ContextCompat.getColor(getApplicationContext(), android.R.color.white));


Set the text color to white

See above about extending AppCompat activity: I believe you will not need this after changing.
Since it took some time to get the colors under control (and is not all done yet :-) ) I really would like to avoid using colors outside of the theme-inherited ones.

I'll change it, BTW, I'm pretty sure we should no longer use the themes that include action bars, AFAIK toolbars are the way to go.
https://medium.com/@ZakTaccardi/goodbye-actionbar-apis-hello-toolbar-af6ae7b31e5d

I agree, we didn't because of lack of time on our end, perhaps you can open a new PR after this one is merged to take care of the migration? It would be really great!

rosenpin · 2017-05-08T18:04:26Z

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/FwAppInstallerActivity.java

+        //Set up the toolbar
+        Toolbar toolbar = (Toolbar) findViewById(R.id.toolbar);
+        toolbar.setTitleTextColor(ContextCompat.getColor(getApplicationContext(), android.R.color.white));
+        toolbar.setTitle(getResources().getString(R.string.installation_d_d, currentAppIndex, appsCount));


Set the title to installing ($index/$count)

rosenpin · 2017-05-08T18:05:06Z

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/FwAppInstallerActivity.java

+        toolbar.setTitle(getResources().getString(R.string.installation_d_d, currentAppIndex, appsCount));
+        toolbar.setNavigationIcon(VectorDrawableCompat.create(getResources(), R.drawable.ic_arrow_back, null));
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP)
+            toolbar.setElevation(16);


Set elevation only on lollipop+ devices (the fancy shadow)

AppCompat should take care of this as well without explicit version checking.

Got it, right

rosenpin · 2017-05-08T18:05:21Z

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/activities/FwAppInstallerActivity.java

+        toolbar.setNavigationOnClickListener(new View.OnClickListener() {
+            @Override
+            public void onClick(View v) {
+                finish();


Finish the current installation activity when the back button is clicked

rosenpin · 2017-05-08T18:10:02Z

...main/java/nodomain/freeyourgadget/gadgetbridge/activities/appmanager/AppManagerActivity.java

+                for (int i = 0; i < appsCount; i++) {
+                    startIntent.setDataAndType(clipData.getItemAt(i).getUri(), null);
+                    startIntent.putExtra("APPS_COUNT", appsCount);
+                    startIntent.putExtra("APP_INDEX", Math.abs(i - appsCount));


Because we want it to be (1/3) -> (2/3) -> (3/3), not (3/3) -> (2/3) -> (1/3)
Why not use for (int i = appsCount; i >=0 ; i--) you ask?
That would be a good question.

rosenpin · 2017-05-08T18:10:41Z

...rc/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/pebble/PebbleIoThread.java

            GB.updateInstallNotification(getContext().getString(R.string.installation_failed_), false, 0, getContext());
        } else {
+            resultIntent = new Intent("nodomain.freeyourgadget.gadgetbridge.INSTALLATION_SUCCESS");


The installation was successful, set the intent action to success

rosenpin · 2017-05-08T18:10:51Z

...rc/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/pebble/PebbleIoThread.java

        if (!mIsInstalling) {
            return;
        }
        if (hadError) {
+            resultIntent = new Intent("nodomain.freeyourgadget.gadgetbridge.INSTALLATION_FAILED");


The installation was unsuccessful, set the intent action to failed

rosenpin · 2017-05-08T18:10:59Z

...rc/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/pebble/PebbleIoThread.java

@@ -646,6 +649,7 @@ private void finishInstall(boolean hadError) {
                }
            }
        }
+        getContext().sendBroadcast(resultIntent);


Send the broadcast

rosenpin · 2017-05-08T18:12:02Z

app/src/main/res/layout/activity_appinstaller.xml

@@ -1,77 +1,69 @@
-<RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android"
+<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"


This layout is pretty much a list of views that goes from top to bottom, no reason to use RelativeLayout with so many rules when we can simply use a vertical LinearLayout

rosenpin · 2017-05-09T16:01:46Z

Also, if you think it's a good idea, I can easily make the selected file install automatically without the need to click the install button and then the back button every time

cpfeiffer · 2017-05-09T19:09:40Z

Sweet!
I just tested Mi Band firmware installation. Installation worked, but the progress notification never finishes. Might well be that this is unrelated to this PR though.

rosenpin · 2017-05-09T21:50:17Z

Oops, that makes sense, as I only made it so the PebbleIoThread sends a broadcast when the installation finishes/fails.
Couldn't find any other devices with installation "trackers" already in place.
The progress bar should be hidden when any non pebble device is used.

danielegobbetti · 2017-05-10T07:04:19Z

@rosenpin I believe cf1451bde5f51678ff19b57fb9da5157a0668dcd should be reverted. As it is now the theme would remain dark until activity is destroyed / application is restarted.

The rest of the theming is very well done, so +1 from my side (with the aforementioned observation).

I'm wondering if the multiple file selection should be limited to pbw (apps), as it would not work (but could be dangerous if it worked) for firmwares.

Removed theme changer commit

rosenpin · 2017-05-10T13:12:55Z

I'm wondering if the multiple file selection should be limited to pbw (apps), as it would not work (but could be dangerous if it worked) for firmwares.

Good point, though it might be a bit confusing for users if the app won't explain the special behavior, maybe there should be different installations screen for apps/software?

rosenpin · 2017-05-10T15:09:27Z

@danielegobbetti
Unrelated question: is it currently possible to filter/blacklist certain apps from posting notifications? If it isn't I can implement this feature, if it is, please let me know how to do this because I couldn't find an option to do so

ashimokawa · 2017-05-10T16:20:05Z

Yes there is a blacklist feature in settings

rosenpin · 2017-05-11T10:45:05Z

Thanks!

cpfeiffer · 2017-05-18T21:46:18Z

So how do we proceed with this?

rosenpin added 4 commits May 8, 2017 16:22

Added option to intall multiple apps at once

5eb8ecc

Improved installation activity UI, Added installation result in the i…

b154288

…nstallation activity

remove redundant statement

2522e25

Added a few comments

ca88474

rosenpin commented May 8, 2017

View reviewed changes

rosenpin added 2 commits May 9, 2017 18:44

Merged no action bar theme and toolbar.xml

3058670

Removes redundant statement to set toolbar elevation

0cb0bc0

rosenpin added 2 commits May 10, 2017 16:05

Removed theme change if the theme is already set

3d96160

Removed theme changer commit

Revert to 0cb0bc0

a8d6024

Merge branch 'master' into master

1dbc22c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

App installation screen improvements #680

App installation screen improvements #680

rosenpin commented May 8, 2017 •

edited

danielegobbetti commented May 8, 2017

rosenpin May 8, 2017

rosenpin May 8, 2017

danielegobbetti May 9, 2017

rosenpin May 9, 2017

rosenpin May 9, 2017

rosenpin May 8, 2017

rosenpin May 8, 2017

danielegobbetti May 9, 2017

rosenpin May 9, 2017

danielegobbetti May 10, 2017

rosenpin May 8, 2017

rosenpin May 8, 2017

danielegobbetti May 9, 2017

rosenpin May 9, 2017

rosenpin May 8, 2017

rosenpin May 8, 2017

rosenpin May 8, 2017

rosenpin May 8, 2017

rosenpin May 8, 2017

rosenpin May 8, 2017

rosenpin commented May 9, 2017

cpfeiffer commented May 9, 2017

rosenpin commented May 9, 2017 •

edited

danielegobbetti commented May 10, 2017 •

edited

rosenpin commented May 10, 2017

rosenpin commented May 10, 2017

ashimokawa commented May 10, 2017

rosenpin commented May 11, 2017

cpfeiffer commented May 18, 2017

		@@ -1,77 +1,69 @@
		<RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android"
		<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"

App installation screen improvements #680

Are you sure you want to change the base?

App installation screen improvements #680

Conversation

rosenpin commented May 8, 2017 • edited

danielegobbetti commented May 8, 2017

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rosenpin commented May 9, 2017

cpfeiffer commented May 9, 2017

rosenpin commented May 9, 2017 • edited

danielegobbetti commented May 10, 2017 • edited

rosenpin commented May 10, 2017

rosenpin commented May 10, 2017

ashimokawa commented May 10, 2017

rosenpin commented May 11, 2017

cpfeiffer commented May 18, 2017

rosenpin commented May 8, 2017 •

edited

rosenpin commented May 9, 2017 •

edited

danielegobbetti commented May 10, 2017 •

edited