@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Improve Differential behavior when scrolling with anchors
Summary:
Fixes T12779. Currently, we scroll down if the midline of the changeset is above the midline of the viewport.
This rule can cause us to scroll improperly when loading changesets //after// jumping to their anchors, since the changeset we want to look at will likely have a midpoint above the document midline. That is, we follow an anchor to `X.c`, then it loads, then we scroll past it.
Instead, scroll only if the changeset is (almost) entirely above the viewport.
Test Plan:
Followed an anchor to `PHUIFeedStoryExample`:
{F4984154}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T12779
Differential Revision: https://secure.phabricator.com/D18052
+14
-8
+14
-8
webroot/rsrc/js/application/diff/DiffChangeset.js
+14
-8
webroot/rsrc/js/application/diff/DiffChangeset.js
···
501
501
// diff with a large number of changes don't constantly have the text
502
502
// area scrolled off the bottom of the screen until the entire diff loads.
503
503
//
504
-
// There are two three major cases here:
504
+
// There are several major cases here:
505
505
//
506
506
// - If we're near the top of the document, never scroll.
507
-
// - If we're near the bottom of the document, always scroll.
508
-
// - Otherwise, scroll if the changes were above the midline of the
509
-
// viewport.
507
+
// - If we're near the bottom of the document, always scroll, unless
508
+
// we have an anchor.
509
+
// - Otherwise, scroll if the changes were above (or, at least,
510
+
// almost entirely above) the viewport.
511
+
//
512
+
// We don't scroll if the changes were just near the top of the viewport
513
+
// because this makes us scroll incorrectly when an anchored change is
514
+
// visible. See T12779.
510
515
511
516
var target = this._node;
512
517
···
529
534
530
535
var target_pos = JX.Vector.getPos(target);
531
536
var target_dim = JX.Vector.getDim(target);
532
-
var target_mid = (target_pos.y + (target_dim.y / 2));
537
+
var target_bot = (target_pos.y + target_dim.y);
533
538
534
-
var view_mid = (old_pos.y + (old_view.y / 2));
535
-
var above_mid = (target_mid < view_mid);
539
+
// Detect if the changeset is entirely (or, at least, almost entirely)
540
+
// above us.
541
+
var above_screen = (target_bot < old_pos.y + 128);
536
542
537
543
var frame = this._getContentFrame();
538
544
JX.DOM.setContent(frame, JX.$H(response.changeset));
539
545
540
546
if (this._stabilize) {
541
547
if (!near_top) {
542
-
if (near_bot || above_mid) {
548
+
if (near_bot || above_screen) {
543
549
// Figure out how much taller the document got.
544
550
var delta = (JX.Vector.getDocument().y - old_dim.y);
545
551
JX.DOM.scrollToPosition(old_pos.x, old_pos.y + delta);