search

wordpress-mobile / WordPress-iOS

WordPress for iOS - Official repository
http://ios.wordpress.org/
GNU General Public License v2.0
3.63k stars 1.1k forks source link

App not always scrolling to correct comment in notifications #16897

Open guarani opened 2 years ago

guarani commented 2 years ago

Expected behavior

When tapping a notification about a comment, tapping on the comment should make the app should scroll all the way to the comment, even if it is in a long thread of comments.

Actual behavior

When tapping a notification, the app doesn’t always scroll all the way to the comment if it’s a long way down (e.g. near the end of a comment thread or project thread). I have seen it fail to scroll all the way, sometimes stopping midway down the thread. Other times it does scroll to the correct comment.

This scrolling issue was a known bug but was addressed in https://github.com/wordpress-mobile/WordPress-iOS/pull/16802 which has the 17.8 milestone. So I’m bringing it up here in case there’s a lingering bug.

Steps to reproduce the behavior

  1. Locate a notification about a comment (e.g. another user liked your comment)
  2. Tap the notification in the app's Notifications tab and be taken to the "Likes" screen
  3. Tap the comment itself and be taken to the "Comments" screen
  4. Notice that the app doesn't scroll you down to the comment in question, sometimes it scrolls half-way, for example

I was able to reproduce this 2 out of 2 times; however, when I went back to try it again and could only reproduce 1 out of 5 times.

Tested on iPhone 11, iOS 14.5, WPiOS 17.8 beta
dangermattic commented 4 months ago

Thanks for reporting! 👍

justtwago commented 4 months ago

My observations and challenges while working on the issue :point_down: I was able to constantly reproduce the issue for very long comments that exceeded the screen size, and it also randomly occurred for shorter posts. It seemed that the list was not fully loaded, and the auto-scroll slid down to the last available comment - that was my assumption, but according to the code we requested the scroll using a comment's index, which should crash the app if there was no such element on the list. This confused me. I tried to add a delay to give the list enough time to load all the comments, but it didn't work. My final assumption is that there is a thread race here between loading and scrolling the list. I tried to use controllerDidChangeContent to scroll the list after the table view renders the elements to prevent a potential race, but I couldn't find a way to trigger this callback.

justtwago commented 1 month ago

Sadly, my second attempt didn't work out: https://github.com/wordpress-mobile/WordPress-iOS/pull/23255. I tried to guarantee scrolling to the right position by enabling the scroll mechanism for each page load, but even after this implementation, some comments didn't properly pop up.

guarani commented 1 month ago

This issue is mentioned in this app feedback: p1717275603690109-slack-C06AJCED8EN

When I try to reply to a comment it doesn't take me to the comment. Rather, it takes me to the list of comments and I have to scroll until I find it. It's much easier in a browser.

justtwago commented 1 month ago

I've just merged a fix for that: https://github.com/wordpress-mobile/WordPress-iOS/pull/23312 and will keep monitoring whether there are any regression or other related issues.