bingle-admin wrote:

The WMF core features team tracks this bug on Mingle card https://mingle.corp.wikimedia.org/projects/flow/cards/651, but people from the community are welcome to contribute here and in Gerrit.

shahyar+wmfbugzilla wrote:

So, what exactly do we want the solution to be here? I looked at the code, and it's rather complex for something as simple as scrolling an element into view.

There seems to be a few options:

1. Always scroll top of element to top of window.
2. Always scroll top of element to middle of window.
3. Scroll top of element to top of window only if element.height > window.height, otherwise scroll middle of element to middle of window.
4. Scroll top of element to middle of window only if element.height > window.height, otherwise scroll middle of element to middle of window.

As far as smoothness goes, jQuery stopped using requestAnimationFrame in 1.6.1. We could probably write something brief to do the scrolling via requestAnimationFrame, and fallback via this method: http://my.opera.com/emoller/blog/2011/12/20/requestanimationframe-for-smart-er-animating -- gotta trust a guy named Erik Möller!

Could we get a list of all (or representative) actions that use this code?

I assume it's things like "when we hit reply to a post" - in which case we'd want to keep (if possible) the post-being-replied-to on-screen as well as scrolling the reply-text-area into view.

shahyar+wmfbugzilla wrote:

(In reply to comment #3)

Could we get a list of all (or representative) actions that use this code?

1. Upon submit, $newRegion.scrollIntoView to go to the new content.
2. When "reply" is clicked, $formContainer.scrollIntoView to go to the reply form container.
3. In function highlightPost, which is used if the URL contains a hash and when icon-permalink is clicked.

Incidentally, this is NOT used when:

1. An error is displayed above or below the form -- and out of the viewport, often.
2. Preview is rendered and is out of the viewport.

The way this works seems really backwards. I'd much prefer to not scroll at ALL unless the new element is out of the viewport, in which case we should perform the minimum amount of scrolling to bring it into the viewport (at the top or at the bottom, depending on current scroll position).

What do you guys think?

Adding werdna to the bug since he implemented the code.

Another action that _should_ use this code is bug 59834 "Preview, Cancel, Reply/Submit do not scroll post into view" (e.g. when you shorten a really long & tall post). That action should also perform the minimal amount of scrolling.

element.scrollIntoView() has certain semantics across all browsers, so we should come up with new function name(s) for other behavior.

(In reply to comment #4)

What do you guys think?

Per the email discussion in "[E2] Scroll Behavior" your proposed UX seems ideal.

Change 107580 had a related patch set uploaded by Legoktm:
refs 58880 - Implement better scrolling via conditionalScroll Refactor ui.js to a cleaner and simpler format

https://gerrit.wikimedia.org/r/107580

Change 107580 merged by jenkins-bot:
Implement better scrolling via conditionalScroll

https://gerrit.wikimedia.org/r/107580