-
Notifications
You must be signed in to change notification settings - Fork 4.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
Classic block: Add scroll to last edit position #23544
Conversation
Size Change: -404 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
… when user returns from editing dialogs
d856fe5
to
d319662
Compare
This tests ok for me. After adding a link or image the new element is scrolled into view. As discussed internally, this still feels a little disorientating if the scroll position is being changed from when you made the edit. It is better than having to manually scroll back though. If we have the ability to save the scroll position at the point in time the edit is made and can scroll back to that, it might be a smoother experience. |
I've noticed that the issue doesn't happen on Firefox (77.0.1, on macOS 10.15.5), so this fix would actually introduce the disorientating scroll change that @aaronrobertshaw noticed. The suggestion of keeping track of the original scroll position is much more involved, but I agree that it would also result in a much cleaner experience. |
@Copons - seems to be a strange little chrome bug causing this that I can't figure a way around, so have added a check to only run if node is out of viewport, so it should now only run on Chrome with the link dialog on longer blocks - so at least minimises the disorientation 😄 |
@glendaviesnz During our internal chat you mentioned that |
I've been doing some more testing on this, I think I've got to the root cause. So, the classic block renders TinyMCE inline: rather than being a scrolling iframe, the TinyMCE area is the height of the content it contains. While scrolling looks like it's just being done to the classic block, it's actually the entire block list which is scrolling. Now, when the link popup closes, it calls The easiest fix may actually be in Core, rather than in Gutenberg: changing the @azaozz, @ellatrix: Can you give your thoughts on this issue, and possible fix? |
I've tested this PR works as described across Chrome, Firefox, Safari and Edge. That said, it looks like @pento is onto something regarding the root cause. My vote would be for addressing the cause directly. |
Thanks @pento, makes sense to fix this in core, but given the time that might take to filter back through I have modified this PR to better track the scroll position and correct it in instances where it differs on return of focus. I think we should include this temporary fix until the problem is resolved in core. |
I've re-tested this and it works well. |
👋 @talldan, @ntsekouras, @ellatrix - we are trying to clear a few of the more annoying Classic block bugs ahead of the deprecation of the Calypso classic editor ... so if there was any chance of getting a sign off on this one in the next few days that would be much appreciated - many thanks 😃 |
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! I tested it and looks good!
We can include this workaround for now but I think an issue/fix should be made at Core though, since the root cause is pinpointed by @pento.
thanks again @ntsekouras - have added a note about the core fix to the original issue |
Temporary patch for #14134
Description
Adds a scollTo to the last bookmarked edit position when returning from onBlur caused by link and image edit dialogs
How has this been tested?
Just tested manually so far
Screenshots
Before:
After:
Types of changes
Adds a scrollIntoView call in order to bring selected node into view
Checklist: