App installation screen improvements #680
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,9 +22,13 @@ | |
| import android.content.Intent; | ||
| import android.content.IntentFilter; | ||
| import android.net.Uri; | ||
| import android.os.Build; | ||
| import android.os.Bundle; | ||
| import android.support.graphics.drawable.VectorDrawableCompat; | ||
| import android.support.v4.app.NavUtils; | ||
| import android.support.v4.content.ContextCompat; | ||
| import android.support.v4.content.LocalBroadcastManager; | ||
| import android.support.v7.widget.Toolbar; | ||
| import android.view.MenuItem; | ||
| import android.view.View; | ||
| import android.widget.Button; | ||
|
|
@@ -55,23 +59,39 @@ public class FwAppInstallerActivity extends GBActivity implements InstallActivit | |
|
|
||
| private static final Logger LOG = LoggerFactory.getLogger(FwAppInstallerActivity.class); | ||
| private static final String ITEM_DETAILS = "details"; | ||
| private final List<ItemWithDetails> mItems = new ArrayList<>(); | ||
| private TextView fwAppInstallTextView; | ||
| private Button installButton; | ||
| private Uri uri; | ||
| private GBDevice device; | ||
| private InstallHandler installHandler; | ||
| private boolean mayConnect; | ||
| private ProgressBar mProgressBar; | ||
| private ListView itemListView; | ||
| private ItemWithDetailsAdapter mItemAdapter; | ||
|
|
||
| private ListView detailsListView; | ||
| private ItemWithDetailsAdapter mDetailsItemAdapter; | ||
| private ArrayList<ItemWithDetails> mDetails = new ArrayList<>(); | ||
|
|
||
| private final BroadcastReceiver installationResultReceiver = new BroadcastReceiver() { | ||
| @Override | ||
| public void onReceive(Context context, Intent intent) { | ||
| //Receive installation fail/success intent | ||
| if (mProgressBar != null) | ||
| mProgressBar.setVisibility(View.INVISIBLE); | ||
| if (installButton != null) { | ||
| installButton.setVisibility(View.VISIBLE); | ||
| installButton.setClickable(false); | ||
| if (intent.getAction().equals("nodomain.freeyourgadget.gadgetbridge.INSTALLATION_FAILED")) { | ||
| installButton.setText(getString(R.string.installation_failed_)); | ||
| } else if (intent.getAction().equals("nodomain.freeyourgadget.gadgetbridge.INSTALLATION_SUCCESS")) { | ||
| installButton.setText(getString(R.string.installation_successful)); | ||
| } | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| private final BroadcastReceiver mReceiver = new BroadcastReceiver() { | ||
| @Override | ||
| public void onReceive(Context context, Intent intent) { | ||
|
|
@@ -128,6 +148,12 @@ private void validateInstallation() { | |
| @Override | ||
| protected void onCreate(Bundle savedInstanceState) { | ||
| super.onCreate(savedInstanceState); | ||
| if (GBApplication.isDarkThemeEnabled()) { | ||
|
There was a problem hiding this comment. 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 There was a problem hiding this comment. 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 There was a problem hiding this comment. 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 There was a problem hiding this comment. I still need to check if the dark theme is enabled and change the theme if it is. But the else statement is redundant |
||
| setTheme(R.style.GadgetbridgeThemeDark_NoActionBar); | ||
| } else { | ||
| setTheme(R.style.GadgetbridgeTheme_NoActionBar); | ||
| } | ||
|
|
||
| setContentView(R.layout.activity_appinstaller); | ||
|
|
||
| GBDevice dev = getIntent().getParcelableExtra(GBDevice.EXTRA_DEVICE); | ||
|
|
@@ -165,13 +191,37 @@ public void onClick(View v) { | |
| setInstallEnabled(false); | ||
| installHandler.onStartInstall(device); | ||
| GBApplication.deviceService().onInstallApp(uri); | ||
| mProgressBar.setVisibility(View.VISIBLE); | ||
| IntentFilter intentFilter = new IntentFilter(); | ||
| intentFilter.addAction("nodomain.freeyourgadget.gadgetbridge.INSTALLATION_FAILED"); | ||
| intentFilter.addAction("nodomain.freeyourgadget.gadgetbridge.INSTALLATION_SUCCESS"); | ||
| registerReceiver(installationResultReceiver, intentFilter); | ||
|
There was a problem hiding this comment. Register the receiver to the failed/success installation intent, the receiver updates the views (progress bar/button) |
||
| } | ||
| }); | ||
|
|
||
| uri = getIntent().getData(); | ||
| if (uri == null) { //for "share" intent | ||
| uri = getIntent().getParcelableExtra(Intent.EXTRA_STREAM); | ||
| } | ||
|
|
||
| //Get apps count and app index from calling intent | ||
| int appsCount = getIntent().getIntExtra("APPS_COUNT", 1); | ||
| int currentAppIndex = getIntent().getIntExtra("APP_INDEX", 1); | ||
|
|
||
| //Set up the toolbar | ||
| Toolbar toolbar = (Toolbar) findViewById(R.id.toolbar); | ||
| toolbar.setTitleTextColor(ContextCompat.getColor(getApplicationContext(), android.R.color.white)); | ||
|
There was a problem hiding this comment. Set the text color to white There was a problem hiding this comment. See above about extending AppCompat activity: I believe you will not need this after changing. There was a problem hiding this comment. 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. There was a problem hiding this comment. 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! |
||
| toolbar.setTitle(getResources().getString(R.string.installation_d_d, currentAppIndex, appsCount)); | ||
|
There was a problem hiding this comment. Set the title to installing ($index/$count) |
||
| toolbar.setNavigationIcon(VectorDrawableCompat.create(getResources(), R.drawable.ic_arrow_back, null)); | ||
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) | ||
| toolbar.setElevation(16); | ||
|
There was a problem hiding this comment. Set elevation only on lollipop+ devices (the fancy shadow) There was a problem hiding this comment. AppCompat should take care of this as well without explicit version checking. |
||
| toolbar.setNavigationOnClickListener(new View.OnClickListener() { | ||
| @Override | ||
| public void onClick(View v) { | ||
| finish(); | ||
|
There was a problem hiding this comment. Finish the current installation activity when the back button is clicked |
||
| } | ||
| }); | ||
|
|
||
| installHandler = findInstallHandlerFor(uri); | ||
| if (installHandler == null) { | ||
| setInfoText(getString(R.string.installer_activity_unable_to_find_handler)); | ||
|
|
@@ -220,13 +270,13 @@ protected void onDestroy() { | |
| } | ||
|
|
||
| @Override | ||
| public CharSequence getInfoText() { | ||
| return fwAppInstallTextView.getText(); | ||
| } | ||
|
|
||
| @Override | ||
| public void setInfoText(String text) { | ||
| fwAppInstallTextView.setText(text); | ||
| } | ||
|
|
||
| @Override | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| <vector xmlns:android="http://schemas.android.com/apk/res/android" | ||
| android:width="24dp" | ||
| android:height="24dp" | ||
| android:viewportWidth="24.0" | ||
| android:viewportHeight="24.0"> | ||
| <path | ||
| android:fillColor="#FFFFFF" | ||
| android:pathData="M20,11H7.83l5.59,-5.59L12,4l-8,8 8,8 1.41,-1.41L7.83,13H20v-2z"/> | ||
| </vector> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,77 +1,69 @@ | ||
| <LinearLayout xmlns:android="http://schemas.android.com/apk/res/android" | ||
|
There was a problem hiding this comment. 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 |
||
| xmlns:tools="http://schemas.android.com/tools" | ||
| android:layout_width="match_parent" | ||
| android:layout_height="match_parent" | ||
| android:animateLayoutChanges="true" | ||
| android:orientation="vertical" | ||
| tools:context="nodomain.freeyourgadget.gadgetbridge.activities.FwAppInstallerActivity"> | ||
|
|
||
| <android.support.v7.widget.Toolbar | ||
| android:id="@+id/toolbar" | ||
| android:layout_width="match_parent" | ||
| android:layout_height="?attr/actionBarSize" | ||
| android:background="?attr/colorPrimary" | ||
| android:title="@string/app_name" /> | ||
|
|
||
| <LinearLayout | ||
| android:layout_width="match_parent" | ||
| android:layout_height="wrap_content" | ||
| android:orientation="vertical" | ||
| android:paddingBottom="@dimen/activity_vertical_margin" | ||
| android:paddingLeft="@dimen/activity_horizontal_margin" | ||
| android:paddingRight="@dimen/activity_horizontal_margin" | ||
| android:paddingTop="@dimen/activity_vertical_margin"> | ||
|
|
||
| <ListView | ||
| android:id="@+id/itemListView" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" /> | ||
|
|
||
| <TextView | ||
| android:id="@+id/infoTextView" | ||
| android:layout_width="fill_parent" | ||
| android:layout_height="wrap_content" | ||
| android:layout_gravity="center_horizontal" /> | ||
|
|
||
| <ImageView | ||
| android:id="@+id/fwappStatusIcon" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_gravity="center_horizontal" /> | ||
|
|
||
| <ProgressBar | ||
| android:id="@+id/installProgressBar" | ||
| android:layout_width="40dp" | ||
| android:layout_height="40dp" | ||
| android:layout_gravity="center_horizontal" | ||
| android:indeterminate="true" | ||
| android:visibility="gone" /> | ||
|
|
||
| <Button | ||
| android:id="@+id/installButton" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_gravity="center_horizontal" | ||
| android:layout_marginTop="10dp" | ||
| android:enabled="false" | ||
| android:text="@string/appinstaller_install" /> | ||
|
|
||
| <ListView | ||
| android:id="@+id/detailsListView" | ||
| android:layout_width="match_parent" | ||
| android:layout_height="wrap_content" /> | ||
|
|
||
| <android.widget.Space | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" /> | ||
|
|
||
| </LinearLayout> | ||
| </LinearLayout> | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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