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

feat(appdistribution): add in-app-feedback support #1463

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 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
6 changes: 4 additions & 2 deletions appdistribution/app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,10 @@ dependencies {
// Import the Firebase BoM (see: https://firebase.google.com/docs/android/learn-more#bom)
implementation platform('com.google.firebase:firebase-bom:31.2.0')

// ADD the SDK to the "prerelease" variant only (example)
implementation 'com.google.firebase:firebase-appdistribution-ktx:16.0.0-beta01'
implementation 'com.google.firebase:firebase-appdistribution-api-ktx:16.0.0-beta06'

// ADD the full SDK implementation to the "debug" variant only
debugImplementation 'com.google.firebase:firebase-appdistribution:16.0.0-beta06'

// For an optimal experience using App Distribution, add the Firebase SDK
// for Google Analytics. This is recommended, but not required.
Expand Down
3 changes: 3 additions & 0 deletions appdistribution/app/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
<?xml version="1.0" encoding="utf-8"?>
<manifest xmlns:android="http://schemas.android.com/apk/res/android">

<!-- Used by the Firebase App Distribution SDK to display in app feedback notifications -->
<uses-permission android:name="android.permission.POST_NOTIFICATIONS"/>

<application
android:name="androidx.multidex.MultiDexApplication"
android:allowBackup="true"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,37 @@
package com.google.firebase.appdistributionquickstart.java;

import android.Manifest;
import android.content.pm.PackageManager;
import android.os.Build;
import android.os.Bundle;
import android.widget.Toast;

import androidx.activity.result.ActivityResultLauncher;
import androidx.activity.result.contract.ActivityResultContracts;
import androidx.appcompat.app.AppCompatActivity;
import androidx.core.content.ContextCompat;

import com.google.firebase.appdistribution.FirebaseAppDistribution;
import com.google.firebase.appdistribution.FirebaseAppDistributionException;
import com.google.firebase.appdistribution.InterruptionLevel;
import com.google.firebase.appdistributionquickstart.databinding.ActivityMainBinding;

public class MainActivity extends AppCompatActivity {

private static final String TAG = "AppDistribution-Quickstart";

// Declare the launcher at the top of your Activity/Fragment:
private final ActivityResultLauncher<String> requestPermissionLauncher =
registerForActivityResult(new ActivityResultContracts.RequestPermission(), isGranted -> {
if (!isGranted) {
Toast.makeText(
MainActivity.this,
"The app won't display feedback notifications because the notification permission was denied",
thatfiredev marked this conversation as resolved.
Show resolved Hide resolved
Toast.LENGTH_LONG
).show();
}
});

private FirebaseAppDistribution mFirebaseAppDistribution;

@Override
Expand All @@ -21,6 +41,19 @@ protected void onCreate(Bundle savedInstanceState) {
setContentView(binding.getRoot());

mFirebaseAppDistribution = FirebaseAppDistribution.getInstance();

binding.btShowNotification.setOnClickListener(view -> {
mFirebaseAppDistribution.showFeedbackNotification(
"Data Collection Notice",
thatfiredev marked this conversation as resolved.
Show resolved Hide resolved
InterruptionLevel.HIGH
);
});

binding.btSendFeedback.setOnClickListener(view -> {
mFirebaseAppDistribution.startFeedback("Thanks for sharing your feedback with us");
thatfiredev marked this conversation as resolved.
Show resolved Hide resolved
});

askNotificationPermission();
thatfiredev marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
Expand All @@ -37,4 +70,29 @@ public void onResume() {
}
});
}

@Override
protected void onDestroy() {
super.onDestroy();
// Hide the notification once this activity is destroyed
mFirebaseAppDistribution.cancelFeedbackNotification();
}
Comment on lines +75 to +80

Choose a reason for hiding this comment

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

You shouldn't need this anymore now that we made the change to automatically hide the notification

Copy link
Member Author

Choose a reason for hiding this comment

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

When does the automatic hide take place? I tried closing the Activity, but it didn't dismiss the notification. Is it when the app is closed?


private void askNotificationPermission() {
// This is only necessary for API level >= 33 (TIRAMISU)
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {

Choose a reason for hiding this comment

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

I don't think you want this check here anymore. Even if they are on an older device we still want to show the notification. On older devices the checkSelfPermission() call will return true.

Copy link
Member Author

Choose a reason for hiding this comment

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

cc: @marinacoelho
The check is due to the POST_NOTIFICATIONS field which was introduced in API 33.
I have added an else branch to display the feedback notification on older devices.

Screenshot 2023-03-07 at 13 12 01

if (ContextCompat.checkSelfPermission(this, Manifest.permission.POST_NOTIFICATIONS) ==
PackageManager.PERMISSION_GRANTED) {
// All set. We can post notifications
} else if (shouldShowRequestPermissionRationale(Manifest.permission.POST_NOTIFICATIONS)) {
// Display an educational UI explaining to the user the features that will be enabled
// by them granting the POST_NOTIFICATION permission. This UI should provide the user
// "OK" and "No thanks" buttons. If the user selects "OK," directly request the permission.
// If the user selects "No thanks," allow the user to continue without notifications.
thatfiredev marked this conversation as resolved.
Show resolved Hide resolved
} else {
// Directly ask for the permission
requestPermissionLauncher.launch(Manifest.permission.POST_NOTIFICATIONS);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
package com.google.firebase.appdistributionquickstart.kotlin

import android.Manifest
import android.content.pm.PackageManager
import android.os.Build
import android.os.Bundle
import android.widget.Toast
import androidx.activity.result.contract.ActivityResultContracts
import androidx.appcompat.app.AppCompatActivity
import androidx.core.content.ContextCompat
import com.google.firebase.appdistribution.FirebaseAppDistribution
import com.google.firebase.appdistribution.FirebaseAppDistributionException
import com.google.firebase.appdistribution.InterruptionLevel
import com.google.firebase.appdistribution.ktx.appDistribution
import com.google.firebase.appdistributionquickstart.databinding.ActivityMainBinding
import com.google.firebase.ktx.Firebase
Expand All @@ -12,12 +19,38 @@ class KotlinMainActivity : AppCompatActivity() {

private lateinit var firebaseAppDistribution: FirebaseAppDistribution

// Declare the launcher at the top of your Activity/Fragment:
private val requestPermissionLauncher = registerForActivityResult(
ActivityResultContracts.RequestPermission()
) { isGranted: Boolean ->
if (!isGranted) {
Toast.makeText(
this@KotlinMainActivity,
"The app won't display feedback notifications because the notification permission was denied",
Toast.LENGTH_LONG
).show()
}
}

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
val binding = ActivityMainBinding.inflate(layoutInflater)
setContentView(binding.root)

firebaseAppDistribution = Firebase.appDistribution

binding.btShowNotification.setOnClickListener {
firebaseAppDistribution.showFeedbackNotification(
"Data Collection Notice",
InterruptionLevel.HIGH
)
}

binding.btSendFeedback.setOnClickListener {
firebaseAppDistribution.startFeedback("Thanks for sharing your feedback with us")
}

askNotificationPermission()
}

override fun onResume() {
Expand All @@ -34,7 +67,30 @@ class KotlinMainActivity : AppCompatActivity() {
}
}

override fun onDestroy() {
super.onDestroy()
// Hide the notification once this activity is destroyed
firebaseAppDistribution.cancelFeedbackNotification()
Copy link
Contributor

Choose a reason for hiding this comment

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

According to Lee's comment above, I think we don't need this line here as well.

}

private fun askNotificationPermission() {
// This is only necessary for API level >= 33 (TIRAMISU)
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
Copy link
Contributor

@marinacoelho marinacoelho Mar 7, 2023

Choose a reason for hiding this comment

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

Same as Lee's comment above. And if we absolutely need to keep this check here, what do you think of changing this function a bit to use when instead of if / else if / else ? (Or switch / case on the Java file).

Copy link
Member Author

Choose a reason for hiding this comment

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

Having a when here might work for Kotlin, since the language allows usage of when without a subject. But not sure if Java has a switch-case without a subject 🤔

if (ContextCompat.checkSelfPermission(this, Manifest.permission.POST_NOTIFICATIONS) ==
PackageManager.PERMISSION_GRANTED
) {
// All set. We can post notifications
} else if (shouldShowRequestPermissionRationale(Manifest.permission.POST_NOTIFICATIONS)) {
// Display an educational UI explaining to the user the features that will be enabled
// by them granting the POST_NOTIFICATION permission. This UI should provide the user
// "OK" and "No thanks" buttons. If the user selects "OK," directly request the permission.
// If the user selects "No thanks," allow the user to continue without notifications.
} else {
// Directly ask for the permission
requestPermissionLauncher.launch(Manifest.permission.POST_NOTIFICATIONS)
}
}
}

companion object {

Expand Down
24 changes: 23 additions & 1 deletion appdistribution/app/src/main/res/layout/activity_main.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
android:layout_width="match_parent"
android:layout_height="match_parent"
android:padding="16dp"
tools:context=".MainActivity">
tools:context=".kotlin.KotlinMainActivity">

<ImageView
android:id="@+id/imageView"
Expand Down Expand Up @@ -34,4 +34,26 @@
app:layout_constraintRight_toRightOf="parent"
app:layout_constraintTop_toBottomOf="@+id/imageView" />

<Button
android:id="@+id/btShowNotification"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginStart="16dp"
android:layout_marginTop="24dp"
android:layout_marginEnd="16dp"
android:text="@string/show_feedback_notification"
app:layout_constraintEnd_toEndOf="@+id/textView"
app:layout_constraintStart_toStartOf="@+id/textView"
app:layout_constraintTop_toBottomOf="@+id/textView" />

<Button
android:id="@+id/btSendFeedback"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginTop="16dp"
android:text="@string/send_feedback"
app:layout_constraintEnd_toEndOf="@+id/btShowNotification"
app:layout_constraintStart_toStartOf="@+id/btShowNotification"
app:layout_constraintTop_toBottomOf="@+id/btShowNotification" />

</androidx.constraintlayout.widget.ConstraintLayout>
2 changes: 2 additions & 0 deletions appdistribution/app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@
<string name="textview_text">Welcome to the Firebase App Distribution Quickstart app. Press the button to trigger an analytics event!</string>

<string name="installation_id_fmt">Firebase Installation ID: %s</string>
<string name="show_feedback_notification">Show Feedback Notification</string>
<string name="send_feedback">Send Feedback</string>
</resources>