Skip to content

Commit 14c2e8c

Browse files
authored
Fix RecyclerView Position Handling in ReaderCommentAdapter (#22048)
* Fix RecyclerView position handling in ReaderCommentAdapter Resolves the "Do not treat position as fixed" lint warning by properly handling adapter positions in onBindViewHolder() and all click listeners. Changes: - Use holder.getBindingAdapterPosition() instead of position parameter for initial data access - Update all click listeners to get fresh position data instead of capturing stale comment objects - Add proper RecyclerView.NO_POSITION validation throughout - Use unique variable names to avoid conflicts (authorCurrentPosition, replyCurrentPosition, etc.) This ensures that comment interactions work correctly even when RecyclerView data changes during the view binding lifecycle. * Improve author click listener logic in ReaderCommentAdapter - Always attach author click listener instead of conditionally based on bind-time data - Move hasAuthorBlogId() check to tap time with fresh comment data - Remove duplicate setOnClickListener call - Ensure consistent pattern with other click handlers that validate data at tap time This is more robust since RecyclerView can recycle views and comment data may change between bind time and tap time.
1 parent 798b9dc commit 14c2e8c

File tree

1 file changed

+59
-28
lines changed

1 file changed

+59
-28
lines changed

WordPress/src/main/java/org/wordpress/android/ui/reader/adapters/ReaderCommentAdapter.java

Lines changed: 59 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,12 @@ public void onClick(View view) {
292292
return;
293293
}
294294

295-
final ReaderComment comment = getItem(position);
295+
int currentPosition = holder.getBindingAdapterPosition();
296+
if (currentPosition == RecyclerView.NO_POSITION) {
297+
return;
298+
}
299+
300+
ReaderComment comment = getItem(currentPosition);
296301
if (comment == null) {
297302
return;
298303
}
@@ -311,26 +316,26 @@ public void onClick(View view) {
311316
String avatarUrl = WPAvatarUtils.rewriteAvatarUrl(comment.getAuthorAvatar(), mAvatarSz);
312317
mImageManager.loadIntoCircle(commentHolder.mImgAvatar, ImageType.AVATAR, avatarUrl);
313318

314-
// tapping avatar or author name opens blog preview
315-
if (comment.hasAuthorBlogId()) {
316-
View.OnClickListener authorListener = new View.OnClickListener() {
317-
@Override
318-
public void onClick(View view) {
319+
View.OnClickListener authorListener = new View.OnClickListener() {
320+
@Override
321+
public void onClick(View view) {
322+
int authorCurrentPosition = commentHolder.getBindingAdapterPosition();
323+
if (authorCurrentPosition == RecyclerView.NO_POSITION) return;
324+
325+
ReaderComment currentComment = getItem(authorCurrentPosition);
326+
// tapping avatar or author name opens blog preview
327+
if (currentComment != null && currentComment.hasAuthorBlogId()) {
319328
ReaderActivityLauncher.showReaderBlogPreview(
320329
view.getContext(),
321-
comment.authorBlogId,
330+
currentComment.authorBlogId,
322331
mPost.isFollowedByCurrentUser,
323332
ReaderTracker.SOURCE_COMMENT,
324333
mReaderTracker
325334
);
326335
}
327-
};
328-
commentHolder.mAuthorContainer.setOnClickListener(authorListener);
329-
commentHolder.mAuthorContainer.setOnClickListener(authorListener);
330-
} else {
331-
commentHolder.mAuthorContainer.setOnClickListener(null);
332-
commentHolder.mAuthorContainer.setOnClickListener(null);
333-
}
336+
}
337+
};
338+
commentHolder.mAuthorContainer.setOnClickListener(authorListener);
334339

335340
// author name uses different color for comments from the post's author
336341
if (comment.authorId == mPost.authorId) {
@@ -382,17 +387,30 @@ public void onClick(View view) {
382387
menuPopup.setAnchorView(commentHolder.mActionButton);
383388
menuPopup.setModal(true);
384389
menuPopup.setOnItemClickListener((parent, view, position1, id) -> {
385-
mCommentMenuActionListener
386-
.onCommentMenuItemTapped(comment, actions.get(position1).getType());
390+
int menuCurrentPosition = commentHolder.getBindingAdapterPosition();
391+
if (menuCurrentPosition != RecyclerView.NO_POSITION) {
392+
ReaderComment currentComment = getItem(menuCurrentPosition);
393+
if (currentComment != null) {
394+
mCommentMenuActionListener
395+
.onCommentMenuItemTapped(currentComment, actions.get(position1).getType());
396+
}
397+
}
387398
menuPopup.dismiss();
388399
});
389400
menuPopup.show();
390401
});
391402
} else {
392403
commentHolder.mActionButton.setImageResource(R.drawable.ic_share_white_24dp);
393-
commentHolder.mActionButtonContainer.setOnClickListener(
394-
v -> mCommentMenuActionListener
395-
.onCommentMenuItemTapped(comment, ReaderCommentMenuActionType.SHARE));
404+
commentHolder.mActionButtonContainer.setOnClickListener(v -> {
405+
int shareCurrentPosition = commentHolder.getBindingAdapterPosition();
406+
if (shareCurrentPosition != RecyclerView.NO_POSITION) {
407+
ReaderComment currentComment = getItem(shareCurrentPosition);
408+
if (currentComment != null) {
409+
mCommentMenuActionListener
410+
.onCommentMenuItemTapped(currentComment, ReaderCommentMenuActionType.SHARE);
411+
}
412+
}
413+
});
396414
}
397415

398416
// show indentation spacer for comments with parents and indent it based on comment level
@@ -441,8 +459,12 @@ public void onClick(View view) {
441459
commentHolder.mReplyView.setOnClickListener(new View.OnClickListener() {
442460
@Override
443461
public void onClick(View v) {
444-
if (mReplyListener != null) {
445-
mReplyListener.onRequestReply(comment.commentId);
462+
int replyCurrentPosition = commentHolder.getBindingAdapterPosition();
463+
if (replyCurrentPosition == RecyclerView.NO_POSITION) return;
464+
465+
ReaderComment currentComment = getItem(replyCurrentPosition);
466+
if (currentComment != null && mReplyListener != null) {
467+
mReplyListener.onRequestReply(currentComment.commentId);
446468
}
447469
}
448470
});
@@ -461,11 +483,11 @@ public void run() {
461483
}
462484
}
463485

464-
showLikeStatus(commentHolder, position);
486+
showLikeStatus(commentHolder);
465487

466488
// if we're nearing the end of the comments and we know more exist on the server,
467489
// fire request to load more
468-
if (mMoreCommentsExist && mDataRequestedListener != null && (position >= getItemCount() - NUM_HEADERS)) {
490+
if (mMoreCommentsExist && mDataRequestedListener != null && (currentPosition >= getItemCount() - NUM_HEADERS)) {
469491
mDataRequestedListener.onRequestData();
470492
}
471493
}
@@ -495,7 +517,12 @@ public void refreshPost() {
495517
}
496518
}
497519

498-
private void showLikeStatus(final CommentHolder holder, int position) {
520+
private void showLikeStatus(final CommentHolder holder) {
521+
int position = holder.getBindingAdapterPosition();
522+
if (position == RecyclerView.NO_POSITION) {
523+
return;
524+
}
525+
499526
ReaderComment comment = getItem(position);
500527
if (comment == null) {
501528
return;
@@ -515,8 +542,7 @@ private void showLikeStatus(final CommentHolder holder, int position) {
515542
holder.mCountLikes.setOnClickListener(new View.OnClickListener() {
516543
@Override
517544
public void onClick(View v) {
518-
int clickedPosition = holder.getBindingAdapterPosition();
519-
toggleLike(v.getContext(), holder, clickedPosition);
545+
toggleLike(v.getContext(), holder);
520546
}
521547
});
522548
}
@@ -526,11 +552,16 @@ public void onClick(View v) {
526552
}
527553
}
528554

529-
private void toggleLike(Context context, CommentHolder holder, int position) {
555+
private void toggleLike(Context context, CommentHolder holder) {
530556
if (!NetworkUtils.checkConnection(context)) {
531557
return;
532558
}
533559

560+
int position = holder.getBindingAdapterPosition();
561+
if (position == RecyclerView.NO_POSITION) {
562+
return;
563+
}
564+
534565
ReaderComment comment = getItem(position);
535566
if (comment == null) {
536567
ToastUtils.showToast(context, R.string.reader_toast_err_generic);
@@ -548,7 +579,7 @@ private void toggleLike(Context context, CommentHolder holder, int position) {
548579
ReaderComment updatedComment = ReaderCommentTable.getComment(comment.blogId, comment.postId, comment.commentId);
549580
if (updatedComment != null) {
550581
mComments.set(position - NUM_HEADERS, updatedComment);
551-
showLikeStatus(holder, position);
582+
showLikeStatus(holder);
552583
}
553584

554585
mReaderTracker.trackPost(isAskingToLike

0 commit comments

Comments
 (0)