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

Disable local notifications with Notification Settings switch #17496

Merged
merged 12 commits into from
Nov 22, 2022
Merged
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

21.3
-----
* [*] Disable local notifications with Notification Settings switch. [https://github.com/wordpress-mobile/WordPress-Android/pull/17496]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be marked as [internal]

Copy link
Member Author

Choose a reason for hiding this comment

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

I think an important issue was fixed. This is not only about Jetpack Focus. With this PR, the Notifications Settings switch will affect local notifications too. So, I think it should not be internal.

* [*] [Jetpack-only] Comments: fix a crash in My Site > Comments with Jetpack standalone plugins. [https://github.com/wordpress-mobile/WordPress-Android/pull/17456]

21.2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

import android.app.Application;
import android.content.Context;
import android.content.SharedPreferences;

import androidx.lifecycle.LiveData;
import androidx.preference.PreferenceManager;

import com.tenor.android.core.network.ApiClient;
import com.tenor.android.core.network.ApiService;
Expand Down Expand Up @@ -109,6 +111,11 @@ public abstract class ApplicationModule {
@ContributesAndroidInjector
abstract BasicDialog contributeBasicDialog();

@Provides
public static SharedPreferences provideSharedPrefs(@ApplicationContext Context context) {
return PreferenceManager.getDefaultSharedPreferences(context);
}

@Provides
public static WizardManager<SiteCreationStep> provideWizardManager(
SiteCreationStepsProvider stepsProvider) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ private boolean isGooglePlayServicesAvailable(Activity activity) {

private void scheduleLocalNotifications() {
mCreateSiteNotificationScheduler.scheduleCreateSiteNotificationIfNeeded();
mWeeklyRoundupScheduler.schedule();
mWeeklyRoundupScheduler.scheduleIfNeeded();
}

@Override
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
package org.wordpress.android.ui.prefs.notifications

import android.content.SharedPreferences
import android.os.Bundle
import android.text.TextUtils
import android.view.MenuItem
import android.view.View
import android.widget.CompoundButton
import android.widget.LinearLayout
import android.widget.TextView
import androidx.appcompat.widget.Toolbar
import androidx.preference.PreferenceManager
import dagger.hilt.android.AndroidEntryPoint
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import org.greenrobot.eventbus.EventBus
import org.greenrobot.eventbus.Subscribe
import org.greenrobot.eventbus.ThreadMode.MAIN
import org.wordpress.android.R.id
import org.wordpress.android.R.layout
import org.wordpress.android.R.string
import org.wordpress.android.analytics.AnalyticsTracker
import org.wordpress.android.analytics.AnalyticsTracker.Stat.NOTIFICATION_SETTINGS_APP_NOTIFICATIONS_DISABLED
import org.wordpress.android.analytics.AnalyticsTracker.Stat.NOTIFICATION_SETTINGS_APP_NOTIFICATIONS_ENABLED
import org.wordpress.android.modules.APPLICATION_SCOPE
import org.wordpress.android.ui.LocaleAwareActivity
import org.wordpress.android.ui.notifications.NotificationEvents.NotificationsSettingsStatusChanged
import org.wordpress.android.ui.prefs.notifications.PrefMainSwitchToolbarView.MainSwitchToolbarListener
import org.wordpress.android.ui.prefs.notifications.usecase.UpdateNotificationSettingsUseCase
import javax.inject.Inject
import javax.inject.Named

@AndroidEntryPoint
class NotificationsSettingsActivity : LocaleAwareActivity(), MainSwitchToolbarListener {
Copy link
Contributor

Choose a reason for hiding this comment

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

🎉 Kudos for converting it to Kotlin

@Inject lateinit var updateNotificationSettingsUseCase: UpdateNotificationSettingsUseCase
@Inject @Named(APPLICATION_SCOPE) lateinit var applicationScope: CoroutineScope

private lateinit var messageTextView: TextView
private lateinit var messageContainer: View

private lateinit var sharedPreferences: SharedPreferences
private lateinit var fragmentContainer: View

public override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
setContentView(layout.notifications_settings_activity)
fragmentContainer = findViewById(id.fragment_container)

// Get shared preferences for main switch.
sharedPreferences = PreferenceManager.getDefaultSharedPreferences(this@NotificationsSettingsActivity)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, renaming it to mainSwitchPrefs would remove the need for comment

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, sharedPreferences doesn't have to be a class variable. I moved it to the function. 7df6d32


// Set up primary toolbar
setUpToolbar()
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be renamed to setupPrimaryToolbar() and remove the comment

Copy link
Member Author

Choose a reason for hiding this comment

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

Done! 7df6d32


// Set up main switch
Copy link
Contributor

Choose a reason for hiding this comment

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

comment is redundant here

Copy link
Member Author

Choose a reason for hiding this comment

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

I just copied Java while converting. But I agree. The code is already explaining itself.
Removed! 7df6d32

setUpMainSwitch()

if (savedInstanceState == null) {
@Suppress("DEPRECATION")
fragmentManager.beginTransaction()
.add(id.fragment_container, NotificationsSettingsFragment())
.commit()
}

messageContainer = findViewById(id.notifications_settings_message_container)
messageTextView = findViewById(id.notifications_settings_message)
}

override fun onStop() {
EventBus.getDefault().unregister(this)
super.onStop()
}

override fun onStart() {
super.onStart()
EventBus.getDefault().register(this)
}

override fun onOptionsItemSelected(item: MenuItem): Boolean {
when (item.itemId) {
android.R.id.home -> {
onBackPressed()
return true
}
}
return super.onOptionsItemSelected(item)
}

@Subscribe(threadMode = MAIN)
fun onEventMainThread(event: NotificationsSettingsStatusChanged) {
if (TextUtils.isEmpty(event.message)) {
messageContainer.visibility = View.GONE
} else {
messageContainer.visibility = View.VISIBLE
messageTextView.text = event.message
}
}

/**
* Set up primary toolbar for navigation and search
*/
private fun setUpToolbar() {
val toolbar = findViewById<Toolbar>(id.toolbar_with_search)

toolbar?.let { setSupportActionBar(it) }

supportActionBar?.apply {
setTitle(string.notification_settings)
setDisplayHomeAsUpEnabled(true)
setDisplayShowTitleEnabled(true)
}
}

/**
* Sets up main switch to disable/enable all notification settings
*/
private fun setUpMainSwitch() {
val mainSwitchToolBarView = findViewById<PrefMainSwitchToolbarView>(id.main_switch)
mainSwitchToolBarView.setMainSwitchToolbarListener(this)

// Set main switch state from shared preferences.
val isMainChecked = sharedPreferences.getBoolean(getString(string.wp_pref_notifications_main), true)
mainSwitchToolBarView.loadInitialState(isMainChecked)
hideDisabledView(isMainChecked)
}

override fun onMainSwitchCheckedChanged(buttonView: CompoundButton?, isChecked: Boolean) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This was removed in a previous PR right?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, onMainSwitchCheckedChanged was being called unnecessarily at launch. I fixed that in #17498. It didn't change this class.

applicationScope.launch { updateNotificationSettingsUseCase.updateNotificationSettings(isChecked) }

hideDisabledView(isChecked)

if (isChecked) {
AnalyticsTracker.track(NOTIFICATION_SETTINGS_APP_NOTIFICATIONS_ENABLED)
} else {
AnalyticsTracker.track(NOTIFICATION_SETTINGS_APP_NOTIFICATIONS_DISABLED)
}
}

/**
* Hide view when Notification Settings are disabled by toggling the main switch off.
*
* @param isMainChecked TRUE to hide disabled view, FALSE to show disabled view
*/
private fun hideDisabledView(isMainChecked: Boolean) {
val notificationsDisabledView = findViewById<LinearLayout>(id.notification_settings_disabled_view)
notificationsDisabledView.visibility = if (isMainChecked) View.INVISIBLE else View.VISIBLE
fragmentContainer.visibility = if (isMainChecked) View.VISIBLE else View.GONE
}
}
Loading