Context instead of Activity in SiteSettingsInterface #6823
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
| import android.content.Context; | ||
| import android.content.SharedPreferences; | ||
| import android.database.Cursor; | ||
| import android.os.Handler; | ||
| import android.support.annotation.NonNull; | ||
| import android.support.annotation.Nullable; | ||
| import android.text.Html; | ||
|
|
@@ -66,7 +67,7 @@ | |
| * This class is marked abstract. This is due to the fact that .org (self-hosted) and .com sites | ||
| * expose different API's to query and edit their respective settings (even though the options | ||
| * offered by each is roughly the same). To get an instance of this interface class use the | ||
| * {@link SiteSettingsInterface#getInterface(Context, SiteModel, SiteSettingsListener)} method. | ||
| */ | ||
|
|
||
| public abstract class SiteSettingsInterface { | ||
|
|
@@ -125,7 +126,7 @@ public abstract class SiteSettingsInterface { | |
| * Instantiates the appropriate (self-hosted or .com) SiteSettingsInterface. | ||
| */ | ||
| @Nullable | ||
| public static SiteSettingsInterface getInterface(Context host, SiteModel site, SiteSettingsListener listener) { | ||
| if (host == null || site == null) return null; | ||
|
|
||
| if (SiteUtils.isAccessedViaWPComRest(site)) { | ||
|
|
@@ -193,7 +194,7 @@ public interface SiteSettingsListener { | |
| */ | ||
| protected abstract void fetchRemoteData(); | ||
|
|
||
| protected final Context mContext; | ||
| protected final SiteModel mSite; | ||
| protected final SiteSettingsListener mListener; | ||
| protected final SiteSettingsModel mSettings; | ||
|
|
@@ -205,10 +206,10 @@ public interface SiteSettingsListener { | |
| @Inject SiteStore mSiteStore; | ||
| @Inject Dispatcher mDispatcher; | ||
|
|
||
| protected SiteSettingsInterface(Context host, SiteModel site, SiteSettingsListener listener) { | ||
| ((WordPress) host.getApplicationContext()).component().inject(this); | ||
| mDispatcher.register(this); | ||
| mContext = host; | ||
| mSite = site; | ||
| mListener = listener; | ||
| mSettings = new SiteSettingsModel(); | ||
|
|
@@ -226,9 +227,9 @@ protected void finalize() throws Throwable { | |
|
|
||
| public void saveSettings() { | ||
| SiteSettingsTable.saveSettings(mSettings); | ||
| siteSettingsPreferences(mContext).edit().putString(LANGUAGE_PREF_KEY, mSettings.language).apply(); | ||
| siteSettingsPreferences(mContext).edit().putInt(DEF_CATEGORY_PREF_KEY, mSettings.defaultCategory).apply(); | ||
| siteSettingsPreferences(mContext).edit().putString(DEF_FORMAT_PREF_KEY, mSettings.defaultPostFormat).apply(); | ||
| } | ||
|
|
||
| public @NonNull String getTitle() { | ||
|
|
@@ -248,14 +249,14 @@ public int getPrivacy() { | |
| } | ||
|
|
||
| public @NonNull String getPrivacyDescription() { | ||
| if (mContext != null) { | ||
| switch (getPrivacy()) { | ||
| case -1: | ||
| return mContext.getString(R.string.site_settings_privacy_private_summary); | ||
| case 0: | ||
| return mContext.getString(R.string.site_settings_privacy_hidden_summary); | ||
| case 1: | ||
| return mContext.getString(R.string.site_settings_privacy_public_summary); | ||
| } | ||
| } | ||
| return ""; | ||
|
|
@@ -275,8 +276,8 @@ public int getPrivacy() { | |
|
|
||
| public @NonNull Map<String, String> getFormats() { | ||
| mSettings.postFormats = new HashMap<>(); | ||
| String[] postFormatDisplayNames = mContext.getResources().getStringArray(R.array.post_format_display_names); | ||
| String[] postFormatKeys = mContext.getResources().getStringArray(R.array.post_format_keys); | ||
| // Add standard post format (only for .com) | ||
| mSettings.postFormats.put(STANDARD_POST_FORMAT_KEY, STANDARD_POST_FORMAT); | ||
| // Add default post formats | ||
|
|
@@ -350,8 +351,8 @@ public boolean getShowRelatedPostImages() { | |
| } | ||
|
|
||
| public @NonNull String getRelatedPostsDescription() { | ||
| if (mContext == null) return ""; | ||
| String desc = mContext.getString(getShowRelatedPosts() ? R.string.on : R.string.off); | ||
| return StringUtils.capitalize(desc); | ||
| } | ||
|
|
||
|
|
@@ -388,11 +389,11 @@ public int getCloseAfterPeriodForDescription() { | |
| } | ||
|
|
||
| public @NonNull String getCloseAfterDescriptionForPeriod(int period) { | ||
| if (mContext == null) return ""; | ||
|
|
||
| if (!getShouldCloseAfter()) return mContext.getString(R.string.never); | ||
|
|
||
| return StringUtils.getQuantityString(mContext, R.string.never, R.string.days_quantity_one, | ||
| R.string.days_quantity_other, period); | ||
| } | ||
|
|
||
|
|
@@ -401,16 +402,16 @@ public int getCommentSorting() { | |
| } | ||
|
|
||
| public @NonNull String getSortingDescription() { | ||
| if (mContext == null) return ""; | ||
|
|
||
| int order = getCommentSorting(); | ||
| switch (order) { | ||
| case SiteSettingsInterface.ASCENDING_SORT: | ||
| return mContext.getString(R.string.oldest_first); | ||
| case SiteSettingsInterface.DESCENDING_SORT: | ||
| return mContext.getString(R.string.newest_first); | ||
| default: | ||
| return mContext.getString(R.string.unknown); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -431,10 +432,10 @@ public int getThreadingLevelsForDescription() { | |
| } | ||
|
|
||
| public @NonNull String getThreadingDescriptionForLevel(int level) { | ||
| if (mContext == null) return ""; | ||
|
|
||
| if (level <= 1) return mContext.getString(R.string.none); | ||
| return String.format(mContext.getString(R.string.site_settings_threading_summary), level); | ||
| } | ||
|
|
||
| public boolean getShouldPageComments() { | ||
|
|
@@ -450,14 +451,14 @@ public int getPagingCountForDescription() { | |
| } | ||
|
|
||
| public @NonNull String getPagingDescription() { | ||
| if (mContext == null) return ""; | ||
|
|
||
| if (!getShouldPageComments()) { | ||
| return mContext.getString(R.string.disabled); | ||
| } | ||
|
|
||
| int count = getPagingCountForDescription(); | ||
| return StringUtils.getQuantityString(mContext, R.string.none, R.string.site_settings_paging_summary_one, | ||
| R.string.site_settings_paging_summary_other, count); | ||
| } | ||
|
|
||
|
|
@@ -548,9 +549,9 @@ public boolean getAllowCommentLikes() { | |
| } | ||
|
|
||
| public @NonNull String getKeysDescription(int count) { | ||
| if (mContext == null) return ""; | ||
|
|
||
| return StringUtils.getQuantityString(mContext, R.string.site_settings_list_editor_no_items_text, | ||
| R.string.site_settings_list_editor_summary_one, | ||
| R.string.site_settings_list_editor_summary_other, count); | ||
| } | ||
|
|
@@ -928,9 +929,9 @@ private void loadCachedSettings() { | |
| * Notifies listener that credentials have been validated or are incorrect. | ||
| */ | ||
| private void notifyCredentialsVerifiedOnUiThread(final Exception error) { | ||
| if (mContext == null || mListener == null) return; | ||
|
|
||
| new Handler().post(new Runnable() { | ||
| @Override | ||
| public void run() { | ||
| mListener.onCredentialsValidated(error); | ||
|
|
@@ -939,11 +940,13 @@ public void run() { | |
| } | ||
|
|
||
| protected void notifyFetchErrorOnUiThread(final Exception error) { | ||
| if (mContext == null | ||
| || (mContext instanceof Activity && ((Activity) mContext).isFinishing()) | ||
|
Contributor
There was a problem hiding this comment. Do we really need to check
Contributor
Author
There was a problem hiding this comment. I was only adapting the existing code without changing the logic and yeah, that check is awkward. I removed it with 0247fd6, letting the listener decide whether the Activity (if any) needs to be checked for finishing. It seems that almost all listeners are already checking for Is that what you had in mind @nbradbury ? |
||
| || mListener == null) { | ||
| return; | ||
| } | ||
|
|
||
| new Handler().post(new Runnable() { | ||
| @Override | ||
| public void run() { | ||
| mListener.onFetchError(error); | ||
|
|
@@ -952,11 +955,13 @@ public void run() { | |
| } | ||
|
|
||
| protected void notifySaveErrorOnUiThread(final Exception error) { | ||
| if (mContext == null | ||
| || (mContext instanceof Activity && ((Activity) mContext).isFinishing()) | ||
| || mListener == null) { | ||
| return; | ||
| } | ||
|
|
||
| new Handler().post(new Runnable() { | ||
| @Override | ||
| public void run() { | ||
| mListener.onSaveError(error); | ||
|
|
@@ -968,9 +973,13 @@ public void run() { | |
| * Notifies listener that settings have been updated with the latest remote data. | ||
| */ | ||
| protected void notifyUpdatedOnUiThread() { | ||
| if (mContext == null | ||
| || (mContext instanceof Activity && ((Activity) mContext).isFinishing()) | ||
| || mListener == null) { | ||
| return; | ||
| } | ||
|
|
||
| new Handler().post(new Runnable() { | ||
| @Override | ||
| public void run() { | ||
| mListener.onSettingsUpdated(); | ||
|
|
@@ -982,9 +991,9 @@ public void run() { | |
| * Notifies listener that settings have been saved or an error occurred while saving. | ||
| */ | ||
| protected void notifySavedOnUiThread() { | ||
| if (mContext == null || mListener == null) return; | ||
|
|
||
| new Handler().post(new Runnable() { | ||
| @Override | ||
| public void run() { | ||
| mListener.onSettingsSaved(); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I know this is pre-existing code, but how about using the builder pattern here?
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.
Makes sense, done in c97620d.