-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[Site Design Revamp] Reset the scroll position every time site designs are shown #18795
[Site Design Revamp] Reset the scroll position every time site designs are shown #18795
Conversation
Generated by 🚫 dangerJS |
You can test the changes in WordPress from this Pull Request by:
|
You can test the changes in Jetpack from this Pull Request by:
|
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.
Thank you for your work on this @twstokes 🙇
The code change seems reasonable too me but testing the CI build on an iPhone SE 2020 (iOS 14.7.1) I was still able to reproduce the issue. Scrolling back and forth didn't get me at the start of the screen 🤔
I'm attaching a video.
18795.mov
Thank you @antonis for spotting this! I'll look into it. |
339b8b3
to
3f6ee94
Compare
@antonis would you mind giving this another review when convenient? I believe the superclass was doing some magic with |
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.
Thank you @antonis for spotting this! I'll look into it.
@antonis would you mind giving this another review when convenient? I believe the superclass was doing some magic with
contentSizeWillChange()
so I changed the ordering of the calls. I have tested with an iPhone 13 running iOS 15.5 and an iPhone SE 2nd gen running iOS 14.5 and so far so good. Thank you!
I tested this on an iPhone 11 (physical device) and it is working well for me. The code changes also look good. I'm not sure what the older code was (I didn't see it before the force-push), but these changes work well now. Nice work Tanner 👍
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.
Thanks! Lines 16 and 17 were swapped. |
Fixes #18794
Simulator.Screen.Recording.-.iPhone.13.-.2022-05-31.at.16.40.19.mp4
Simulator.Screen.Recording.-.iPhone.13.-.2022-05-31.at.16.39.15.mp4
Testing:
Regression Notes
PR submission checklist:
RELEASE-NOTES.txt
if necessary.