Awkward jump when clicking text tab after scrolling visual editor

[gcorne]

After scrolling the visual editor, there is an awkward jump when clicking the text tab.

See ​https://cloudup.com/cEY-y18v5_N for a demonstration

[stephdau]

I don't have code specifics yet, but testing seems to lead me to believe it's directly tied to reaching the bottom-most of the page while in visual mode.

One can notice a drastic size difference between the visual and text editors when switching from one to the other while not yet at the bottom of the page. The only context in which I can reproduce Greg's bug is after reaching the bottom edge of the page.

See ​https://cloudup.com/cPfihqdfbV2

[stephdau]

When the bug happens, we can also see that "calculations" meant to keep the sidebar off the footer are also failing.

​https://cloudup.com/c7UYGLHoAp9

[stephdau]

1. Switching from the visual editor to the text editor triggers textEditorResize().
2. The text editor is never as tall as the visual one, with the same content, since same but unstyled.
3. We resize the editor to be the height of the non-WYSIWYG text content (see editor status bar moving up.
4. But we don't (or fail to) recalculate where the top/bottom of the pinned sidebar should be in relation to the current editor's size (I think).

EG: switching hiddenHeight for textEditorHeight at https://core.trac.wordpress.org/browser/trunk/src/wp-admin/js/editor-expand.js?rev=29656#L144 stops the Awkward jump from happening, but the text editor ends with a slew of whitespace at its bottom that the user cannot access (totalling a height equal to the discrepancy between the visual and text editors heights). So not quite a solution.

Another hack to "make the problem go away) is to force a scrollTop(0) when switching to the text editor (aka, in mceHide()), which recalculates the editor and sidebar positions, and make the issue moot. Still not great.

Still currently at a loss for an actual fix. :)
Seems to need another clever // don't get over the footer for the sidebar, upon switching/resize.

[azaozz]

Was thinking about this too: perhaps we can scroll up as many px as the difference between Visual to Text. This should take care of the sidebar position too.

[stephdau]

Good idea, will try.

[stephdau]

Better, but not quite: ​https://cloudup.com/cxJXT5WG4Ar
Which is surprising, because I thought it'd be enough to at least trigger a proper recalculation of the UX element's position (at least the sidebar in relation to the footer).

[stephdau]

So, what's "disturbing" with my suggestion to scroll to the top of the page is that it brings back the title and slug fields back in view. Another solution could be to scroll to the "top of the editor", which can be defined by the bottom of the slug area. This should be invisible to the user, since the editor's toolbar is static, and should meet the height and position requirements of the sidebar.

[stephdau]

Proof of concept for comment:8

[stephdau]

attachment:29338.diff​ is a rough proof of concept (hard-coded position instead of dynamic) of what I suggested in comment:8.

See behavior in ​https://cloudup.com/cW8i7vbaPWR video.

[stephdau]

PS: because what it does is scroll up, the sidebar positioning case that's triggered does make the sidebar pin at the top, not the bottom, which would be ideal, but going against the grain of what needs to happen when actually scrolling up on purpose.

[azaozz]

Replying to stephdau:

Yep, calculating the height difference and scrolling by that much seems to work well too. To fix the sidebar, we need to do the same trick as when toggling postboxes: scroll in one direction, adjust, scroll in the other direction and adjust again. 29338.2.patch seems to work well here.

[azaozz]

29338.3.patch also checks if the difference in heights would mean scrolling all the way to the top of the page and limits it to scrolling to the top of the editor. Fixes this in cases where the Text editor is really short and the Visual is really tall, for example: a gallery post.

[azaozz]

In 29338.4.patch: also add some delay before calculating heights as in some cases the pre-switch heights are returned.

[helen]

Looks good - the slight delay is noticeable if you're looking for it, but it's a lot better than having the editor jump off the screen.

[iseulde]

I still notice some weird end positions... When I have a post full of views and switch to text mode form the bottom, I end up with the text content scrolled just one line away from the top and the toolbar out of sight (the text editor is too small to have the fixed toolbar).

I still think it'd be better to just scroll to the top of the editor whenever the top toolbar is pinned, but the current solution is already way better than it was. :)

[iseulde]

How about this?

[azaozz]

29338.6.patch looks good here. Changes the behavior a bit, now switching Visual => Text will always scroll to the editor top.

[adamsilverstein]

Replying to azaozz:

29338.6.patch looks good here. Changes the behavior a bit, now switching Visual => Text will always scroll to the editor top.

29338.6.patch looks really good, nice!

[iseulde]

I think 29338.6.patch is the way to go. Always scrolling to the top when switching mode makes mosts sense to me, it's impossible to scroll exactly to the corresponding line in the text editor anyway. Switching modes is disorientation in itself.

[nacin]

In 3.9, each editor's current scroll location is remembered. I think scrolling to the top might make it really annoying to do certain things like switching multiple times in a row to adjust something, for example.

[iseulde]

So you want to remember the position for each editor and switch to that position if possible, and otherwise to the top? Sounds good to me too. I'll try making another patch. Can't remember that in 3.9 though.

[iseulde]

Okay, right, in 3.9 the browser remembers the scroll position for the textarea and TinyMCE focuses on the last caret position... Trying to make a patch now...

[iseulde]

New patch: just remember the position of the mode and set that position back after switching back.

[markjaquith]

It's gonna be a little weird if you switch, scroll, then switch back. but I think most switches are either quick "I want to insert some custom HTML here and then go back to writing" or long-lasting switches. Saving the scroll states individually works great for quick diversions into another mode, and doesn't really matter for long-lasting switches.

I would defer to @avryl on the safety of this code change. In my testing, I didn't have any functional problems.

[helen]

There's more redraw here than before with the patch, but it is less jarring than having things jump off the screen. I also agree with Mark that the scroll state saving works out well for the most common use cases.

[nacin]

Would like azaozz to review this, but it can land for the soak.

[helen]

In 29693:

Editor scrolling: Remember and restore scroll position for visual and text modes.

This prevents the editor from jumping off screen when the rendered visual content is significantly taller than plain text, e.g. when there are views present.

This is also a close restoration of 3.9 behavior, some of which was browser-specific.

props avryl.
fixes #29338.

[iseulde]

Needs checks to see if the old position is within the editor's range.

[azaozz]

29338.9.patch reverts 29338.8.patch and applies 29338.6.patch. This always scrolls the Text editor to the top and prevents all other edge cases.

[iseulde]

Looks good! :)

[helen]

Go for it.

For reference, this does lose the text editor scroll position, which was browser-specific behavior (I never saw it in Firefox, but it worked in Chrome). Still supports the use case of flipping quickly into text mode to insert some HTML and going back to visual, which does retain scroll position, quite well.

[azaozz]

In 29698:

Editor expand: revert back to scrolling to the editor top when switching Visual to Text, fixes #29338

[azaozz]

Replying to helen:

For reference, this does lose the text editor scroll position, which was browser-specific behavior (I never saw it in Firefox, but it worked in Chrome).

Yeah, that would be nice to have. Created #29492 for an enhancement in 4.1.