Skip to content
This repository has been archived by the owner on Apr 9, 2021. It is now read-only.

Added App information in about section #350

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions skunkworks_crow/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,5 @@ dependencies {
// Permissions Dispatcher
implementation "org.permissionsdispatcher:permissionsdispatcher:4.5.0"
annotationProcessor "org.permissionsdispatcher:permissionsdispatcher-processor:4.5.0"
implementation 'androidx.browser:browser:1.2.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why introducing a third-party lib for this small feature @devanshi7799 ? If I were you, I will implement that using a simple web view.

Downsides for introducing a new lib in this case are:

  1. Some third-party libs have many redundant functions, that will increase the apk size.
  2. Some third-party libs are not well tested that may introduce bugs that cannot fix.

So we are trying not to use third-party libs unless we have to do that, I think this would be helpful for you.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why introducing a third-party lib for this small feature @devanshi7799 ? If I were you, I will implement that using a simple web view.

Hi @huangyz0918, it's a custom tabs dependency, and nowadays every app has in-app browser feature which provides a look and feel of the browser within the app without actually moving out of the app, so I think its fine we even have this in ODK-Collect but not AndroidX dependency. I think @devanshi7799 this is fine no need to explore the webview as of now, if any feature ask comes in future then we can pick that up or if it we face any issues due to this then we can pick this up.

}
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package org.odk.share.views.ui.about;

import android.content.Intent;
import android.net.Uri;
import android.os.Bundle;
import android.view.View;

import androidx.appcompat.app.AppCompatActivity;
import androidx.appcompat.widget.Toolbar;
import androidx.browser.customtabs.CustomTabsIntent;
import androidx.recyclerview.widget.LinearLayoutManager;
import androidx.recyclerview.widget.RecyclerView;

Expand All @@ -22,6 +24,7 @@ public class AboutActivity extends AppCompatActivity implements OnItemClickListe

private static final String LICENSES_HTML_PATH = "file:///android_asset/open_source_licenses.html";
private static final String USER_GUIDE_HTML_PATH = "file:///android_asset/user_guide.html";
private static final String SKUNKWORKS_CROW_README_URL = "https://github.com/opendatakit/skunkworks-crow/blob/master/README.md";

@BindView(R.id.recyclerview)
RecyclerView recyclerView;
Expand All @@ -30,6 +33,7 @@ public class AboutActivity extends AppCompatActivity implements OnItemClickListe

private AboutAdapter adapter;


@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
Expand All @@ -47,19 +51,25 @@ protected void onCreate(Bundle savedInstanceState) {
llm.setOrientation(RecyclerView.VERTICAL);
recyclerView.setLayoutManager(llm);
adapter = new AboutAdapter(this, this);
adapter.addItem(new AboutItem(R.string.app_information, R.drawable.ic_stars));
adapter.addItem(new AboutItem(R.string.open_source_licenses, R.drawable.ic_stars));
adapter.addItem(new AboutItem(R.string.user_guide, R.drawable.ic_stars));
recyclerView.setAdapter(adapter);
}

@Override
@Override
public void onItemClick(View view, int position) {
if (position == 0) {

new CustomTabsIntent.Builder()
Copy link
Contributor

Choose a reason for hiding this comment

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

My main concern was that what if the device doesn't support custom tabs, it would be easier for you to understand if you can explore ODK-Collect Custom tabs implementation that they have because that handles lot of things which we might not be able to test on our devices, there are even few devices which doesn't have browser in them, so this change may crash the app on those devices.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lakshyagupta21 Should i make changes according to odk-collect custom tabs?

.build()
.launchUrl(this, Uri.parse(SKUNKWORKS_CROW_README_URL));
} else if (position == 1) {
Intent intent = new Intent(this, WebViewActivity.class);
intent.putExtra(OPEN_URL, LICENSES_HTML_PATH);
intent.putExtra(TITLE, getString(R.string.open_source_licenses));
startActivity(intent);
} else if (position == 1) {
} else if (position == 2) {
Intent intent = new Intent(this, WebViewActivity.class);
intent.putExtra(OPEN_URL, USER_GUIDE_HTML_PATH);
intent.putExtra(TITLE, getString(R.string.user_guide));
Expand Down
1 change: 1 addition & 0 deletions skunkworks_crow/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
<string name="about">About</string>
<string name="open_source_licenses">Open Source Licenses</string>
<string name="user_guide">User Guide</string>
<string name="app_information">App Information</string>
<string name="default_hotspot_ssid">hotspot</string>
<string name="default_hotspot_password">12345678</string>
<string name="title_bluetooth_name">Bluetooth Name</string>
Expand Down