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.

Sign up for GitHub

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

[Nullability Annotations to Java Classes] Replace findViewById with ViewBinding for Comments (relatively safe) #19217

Merged
merged 8 commits into from
Sep 21, 2023
Merged

[Nullability Annotations to Java Classes] Replace findViewById with ViewBinding for Comments (relatively safe) #19217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d375f6a
Refactor: Replace findviewbyid with viewbinding for comments detail
ParaskP7 Sep 19, 2023
74f939d
Refactor: Use non-null viewbinding method parameter for comments detail
ParaskP7 Sep 19, 2023
011658b
Analysis: Add missing nullability annotations to sdk override methods
ParaskP7 Sep 19, 2023
91eed09
Analysis: Suppress deprecation warnings on comments detail
ParaskP7 Sep 19, 2023
9ca0a3c
Analysis: Add missing nl-a to status filter field
ParaskP7 Sep 19, 2023
525ca58
Analysis: Add missing nl-a to site field
ParaskP7 Sep 19, 2023
1b709a0
Analysis: Add missing nl-a to adapter field
ParaskP7 Sep 19, 2023
5953977
Analysis: Add missing nl-a to on page change listener field
ParaskP7 Sep 19, 2023
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
5 changes: 4 additions & 1 deletion WordPress/src/main/java/org/wordpress/android/ui/comments/CommentDetailFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,10 @@ public class CommentDetailFragment extends ViewPagerFragment implements Notifica
/*
* used when called from comment list
*/
static CommentDetailFragment newInstance(SiteModel site, CommentModel commentModel) {
static CommentDetailFragment newInstance(
@NonNull SiteModel site,
CommentModel commentModel
) {
CommentDetailFragment fragment = new CommentDetailFragment();
Bundle args = new Bundle();
args.putSerializable(KEY_MODE, CommentSource.SITE_COMMENTS);
Expand Down
13 changes: 8 additions & 5 deletions WordPress/src/main/java/org/wordpress/android/ui/comments/CommentDetailFragmentAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import android.os.Parcelable;

import androidx.annotation.NonNull;
import androidx.fragment.app.Fragment;
import androidx.fragment.app.FragmentManager;
import androidx.fragment.app.FragmentStatePagerAdapter;
Expand All @@ -19,14 +20,16 @@
*/
@Deprecated
public class CommentDetailFragmentAdapter extends FragmentStatePagerAdapter {
private final SiteModel mSite;
@NonNull private final SiteModel mSite;
private final OnLoadMoreListener mOnLoadMoreListener;
private final CommentList mCommentList;

CommentDetailFragmentAdapter(FragmentManager fm,
CommentList commentList,
SiteModel site,
OnLoadMoreListener onLoadMoreListener) {
CommentDetailFragmentAdapter(
FragmentManager fm,
CommentList commentList,
@NonNull SiteModel site,
OnLoadMoreListener onLoadMoreListener
) {
super(fm, BEHAVIOR_RESUME_ONLY_CURRENT_FRAGMENT);
this.mSite = site;
this.mOnLoadMoreListener = onLoadMoreListener;
Expand Down
162 changes: 89 additions & 73 deletions WordPress/src/main/java/org/wordpress/android/ui/comments/CommentsDetailActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,19 @@
import android.text.TextUtils;
import android.view.MenuItem;
import android.view.View;
import android.widget.ProgressBar;

import androidx.activity.OnBackPressedCallback;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.appcompat.app.ActionBar;
import androidx.appcompat.widget.Toolbar;
import androidx.viewpager.widget.ViewPager;

import com.google.android.material.appbar.AppBarLayout;

import org.greenrobot.eventbus.Subscribe;
import org.greenrobot.eventbus.ThreadMode;
import org.wordpress.android.R;
import org.wordpress.android.WordPress;
import org.wordpress.android.analytics.AnalyticsTracker.Stat;
import org.wordpress.android.databinding.CommentsDetailActivityBinding;
import org.wordpress.android.fluxc.action.CommentAction;
import org.wordpress.android.fluxc.generated.CommentActionBuilder;
import org.wordpress.android.fluxc.model.CommentModel;
Expand All @@ -42,7 +39,6 @@
import org.wordpress.android.util.analytics.AnalyticsUtils;
import org.wordpress.android.util.analytics.AnalyticsUtils.AnalyticsCommentActionSource;
import org.wordpress.android.util.extensions.CompatExtensionsKt;
import org.wordpress.android.widgets.WPViewPager;
import org.wordpress.android.widgets.WPViewPagerTransformer;

import javax.inject.Inject;
Expand All @@ -58,35 +54,37 @@
*/
@Deprecated
@AndroidEntryPoint
@SuppressWarnings({"deprecation", "DeprecatedIsStillUsed"})
public class CommentsDetailActivity extends LocaleAwareActivity
implements OnLoadMoreListener,
CommentActions.OnCommentActionListener, ScrollableViewInitializedListener {
public static final String COMMENT_ID_EXTRA = "commentId";
public static final String COMMENT_STATUS_FILTER_EXTRA = "commentStatusFilter";

@SuppressWarnings("deprecation")
@Inject CommentsStoreAdapter mCommentsStoreAdapter;

private WPViewPager mViewPager;
private AppBarLayout mAppBarLayout;
private ProgressBar mProgressBar;

private long mCommentId;
private CommentStatus mStatusFilter;
private SiteModel mSite;
private CommentDetailFragmentAdapter mAdapter;
private ViewPager.OnPageChangeListener mOnPageChangeListener;
@Nullable private CommentStatus mStatusFilter;
@Nullable private SiteModel mSite;
@SuppressWarnings("deprecation")
@Nullable private CommentDetailFragmentAdapter mAdapter;
@Nullable private ViewPager.OnPageChangeListener mOnPageChangeListener;

private boolean mIsLoadingComments;
private boolean mIsUpdatingComments;
private boolean mCanLoadMoreComments = true;

@Nullable private CommentsDetailActivityBinding mBinding = null;

@Override
public void onCreate(@Nullable Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
mCommentsStoreAdapter.register(this);
AppLog.i(AppLog.T.COMMENTS, "Creating CommentsDetailActivity");

setContentView(R.layout.comments_detail_activity);
mBinding = CommentsDetailActivityBinding.inflate(getLayoutInflater());
setContentView(mBinding.getRoot());

OnBackPressedCallback callback = new OnBackPressedCallback(true) {
@Override
Expand All @@ -103,12 +101,13 @@ public void handleOnBackPressed() {
};
getOnBackPressedDispatcher().addCallback(this, callback);

Toolbar toolbar = findViewById(R.id.toolbar_main);
setSupportActionBar(toolbar);
ActionBar actionBar = getSupportActionBar();
if (actionBar != null) {
actionBar.setDisplayShowTitleEnabled(true);
actionBar.setDisplayHomeAsUpEnabled(true);
if (mBinding != null) {
setSupportActionBar(mBinding.toolbarMain);
ActionBar actionBar = getSupportActionBar();
if (actionBar != null) {
actionBar.setDisplayShowTitleEnabled(true);
actionBar.setDisplayHomeAsUpEnabled(true);
}
}

if (savedInstanceState == null) {
Expand All @@ -121,16 +120,14 @@ public void handleOnBackPressed() {
mStatusFilter = (CommentStatus) savedInstanceState.getSerializable(COMMENT_STATUS_FILTER_EXTRA);
}

// set up the viewpager and adapter for lateral navigation
mViewPager = findViewById(R.id.viewpager);
mViewPager.setPageTransformer(false,
new WPViewPagerTransformer(WPViewPagerTransformer.TransformType.SLIDE_OVER));

mProgressBar = findViewById(R.id.progress_loading);
mAppBarLayout = findViewById(R.id.appbar_main);
if (mBinding != null) {
// set up the viewpager and adapter for lateral navigation
mBinding.viewpager.setPageTransformer(false,
new WPViewPagerTransformer(WPViewPagerTransformer.TransformType.SLIDE_OVER));

// Asynchronously loads comments and build the adapter
loadDataInViewPager();
// Asynchronously loads comments and build the adapter
loadDataInViewPager(mBinding);
}

if (savedInstanceState == null) {
// track initial comment view
Expand All @@ -140,7 +137,7 @@ public void handleOnBackPressed() {
}

@Override
protected void onSaveInstanceState(Bundle outState) {
protected void onSaveInstanceState(@NonNull Bundle outState) {
outState.putLong(COMMENT_ID_EXTRA, mCommentId);
outState.putSerializable(WordPress.SITE, mSite);
outState.putSerializable(COMMENT_STATUS_FILTER_EXTRA, mStatusFilter);
Expand All @@ -164,10 +161,12 @@ public boolean onOptionsItemSelected(@NonNull MenuItem item) {

@Override
public void onLoadMore() {
updateComments();
if (mBinding != null) {
updateComments(mBinding);
}
}

private void updateComments() {
private void updateComments(@NonNull CommentsDetailActivityBinding binding) {
if (mIsUpdatingComments) {
AppLog.w(AppLog.T.COMMENTS, "update comments task already running");
return;
Expand All @@ -179,98 +178,112 @@ private void updateComments() {
return;
}

final int offset = mAdapter.getCount();
mCommentsStoreAdapter.dispatch(CommentActionBuilder.newFetchCommentsAction(
new FetchCommentsPayload(mSite, mStatusFilter, COMMENTS_PER_PAGE, offset)));
mIsUpdatingComments = true;
setLoadingState(true);
if (mSite != null && mStatusFilter != null && mAdapter != null) {
mCommentsStoreAdapter.dispatch(CommentActionBuilder.newFetchCommentsAction(
new FetchCommentsPayload(mSite, mStatusFilter, COMMENTS_PER_PAGE, mAdapter.getCount()))
);
mIsUpdatingComments = true;
setLoadingState(binding, true);
}
}

@SuppressWarnings("unused")
@Subscribe(threadMode = ThreadMode.MAIN)
public void onCommentChanged(OnCommentChanged event) {
mIsUpdatingComments = false;
setLoadingState(false);
// Don't refresh the list on push, we already updated comments
if (event.causeOfChange != CommentAction.PUSH_COMMENT) {
if (event.changedCommentsLocalIds.size() > 0) {
loadDataInViewPager();
} else if (!event.isError()) {
// There are no more comments to load
mCanLoadMoreComments = false;
if (mBinding != null) {
mIsUpdatingComments = false;
setLoadingState(mBinding, false);
// Don't refresh the list on push, we already updated comments
if (event.causeOfChange != CommentAction.PUSH_COMMENT) {
if (event.changedCommentsLocalIds.size() > 0) {
loadDataInViewPager(mBinding);
} else if (!event.isError()) {
// There are no more comments to load
mCanLoadMoreComments = false;
}
}
}
if (event.isError()) {
if (!TextUtils.isEmpty(event.error.message)) {
ToastUtils.showToast(this, event.error.message);
if (event.isError()) {
if (!TextUtils.isEmpty(event.error.message)) {
ToastUtils.showToast(this, event.error.message);
}
}
}
}

@SuppressWarnings("deprecation")
private void loadDataInViewPager() {
private void loadDataInViewPager(@NonNull CommentsDetailActivityBinding binding) {
if (mIsLoadingComments) {
AppLog.w(AppLog.T.COMMENTS, "load comments task already active");
} else {
new LoadCommentsTask(mCommentsStoreAdapter, mStatusFilter, mSite, new LoadCommentsTask.LoadingCallback() {
@Override
public void isLoading(boolean loading) {
setLoadingState(loading);
setLoadingState(binding, loading);
mIsLoadingComments = loading;
}

@Override
public void loadingFinished(CommentList commentList) {
if (!commentList.isEmpty()) {
showCommentList(commentList);
showCommentList(binding, commentList);
}
}
}).executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
}
}

private void showCommentList(CommentList commentList) {
@SuppressWarnings("deprecation")
private void showCommentList(
@NonNull CommentsDetailActivityBinding binding,
CommentList commentList
) {
if (isFinishing()) {
return;
}
final int previousItem = mViewPager.getCurrentItem();
final int previousItem = binding.viewpager.getCurrentItem();

// Only notify adapter when loading new page
if (mAdapter != null && mAdapter.isAddingNewComments(commentList)) {
mAdapter.onNewItems(commentList);
} else {
// If current items change, rebuild the adapter
mAdapter = new CommentDetailFragmentAdapter(getSupportFragmentManager(), commentList, mSite,
CommentsDetailActivity.this);
mViewPager.setAdapter(mAdapter);
if (mSite != null) {
// If current items change, rebuild the adapter
mAdapter = new CommentDetailFragmentAdapter(getSupportFragmentManager(), commentList, mSite,
CommentsDetailActivity.this);
binding.viewpager.setAdapter(mAdapter);
} else {
throw new IllegalStateException("mAdapter cannot be constructed; mSite is null");
}
}

final int commentIndex = mAdapter.commentIndex(mCommentId);
if (commentIndex < 0) {
showErrorToastAndFinish();
}
if (mOnPageChangeListener != null) {
mViewPager.removeOnPageChangeListener(mOnPageChangeListener);
binding.viewpager.removeOnPageChangeListener(mOnPageChangeListener);
} else {
mOnPageChangeListener = new ViewPager.SimpleOnPageChangeListener() {
@Override
public void onPageSelected(int position) {
super.onPageSelected(position);
final CommentModel comment = mAdapter.getCommentAtPosition(position);
if (comment != null) {
mCommentId = comment.getRemoteCommentId();
// track subsequent comment views
AnalyticsUtils.trackCommentActionWithSiteDetails(
Stat.COMMENT_VIEWED, AnalyticsCommentActionSource.SITE_COMMENTS, mSite);
if (mAdapter != null) {
final CommentModel comment = mAdapter.getCommentAtPosition(position);
if (comment != null) {
mCommentId = comment.getRemoteCommentId();
// track subsequent comment views
AnalyticsUtils.trackCommentActionWithSiteDetails(
Stat.COMMENT_VIEWED, AnalyticsCommentActionSource.SITE_COMMENTS, mSite);
}
}
}
};
}
if (commentIndex != previousItem) {
mViewPager.setCurrentItem(commentIndex);
binding.viewpager.setCurrentItem(commentIndex);
}

mViewPager.addOnPageChangeListener(mOnPageChangeListener);
binding.viewpager.addOnPageChangeListener(mOnPageChangeListener);
}

private void showErrorToastAndFinish() {
Expand All @@ -279,10 +292,11 @@ private void showErrorToastAndFinish() {
finish();
}

private void setLoadingState(boolean visible) {
if (mProgressBar != null) {
mProgressBar.setVisibility(visible ? View.VISIBLE : View.GONE);
}
private void setLoadingState(
@NonNull CommentsDetailActivityBinding binding,
boolean visible
) {
binding.progressLoading.setVisibility(visible ? View.VISIBLE : View.GONE);
}


Expand All @@ -299,6 +313,8 @@ public void onModerateComment(final SiteModel site,

@Override
public void onScrollableViewInitialized(int containerId) {
mAppBarLayout.setLiftOnScrollTargetViewId(containerId);
if (mBinding != null) {
mBinding.appbarMain.setLiftOnScrollTargetViewId(containerId);
}
}
}
10 changes: 6 additions & 4 deletions WordPress/src/main/java/org/wordpress/android/ui/comments/LoadCommentsTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import android.os.AsyncTask;

import androidx.annotation.Nullable;

import org.wordpress.android.fluxc.model.CommentModel;
import org.wordpress.android.fluxc.model.CommentStatus;
import org.wordpress.android.fluxc.model.SiteModel;
Expand All @@ -24,13 +26,13 @@ interface LoadingCallback {
}

private final CommentsStoreAdapter mCommentsStoreAdapter;
private final CommentStatus mStatusFilter;
private final SiteModel mSite;
@Nullable private final CommentStatus mStatusFilter;
@Nullable private final SiteModel mSite;
private final LoadingCallback mLoadingCallback;

LoadCommentsTask(CommentsStoreAdapter commentsStoreAdapter,
CommentStatus statusFilter,
SiteModel site,
@Nullable CommentStatus statusFilter,
@Nullable SiteModel site,
LoadingCallback loadingCallback) {
this.mCommentsStoreAdapter = commentsStoreAdapter;
this.mStatusFilter = statusFilter;
Expand Down
Loading