Page MenuHomePhabricator

For the diff banner, detect the current changeset better
ClosedPublic

Authored by epriestley on May 20 2017, 11:30 AM.
Tags
None
Referenced Files
F13286819: D17976.diff
Tue, Jun 4, 7:43 AM
F13273756: D17976.diff
Fri, May 31, 2:35 AM
F13244066: D17976.id43241.diff
Thu, May 23, 4:29 AM
F13234430: D17976.diff
Tue, May 21, 3:23 AM
F13212233: D17976.diff
Fri, May 17, 6:30 AM
F13198449: D17976.id.diff
Mon, May 13, 6:40 AM
F13198413: D17976.id43233.diff
Mon, May 13, 6:19 AM
F13195803: D17976.diff
Sun, May 12, 10:38 PM
Subscribers
None

Details

Summary

Ref T12733. Currently, we detect the changeset which is in the middle of the screen as the current changeset.

This doesn't always get us the most intuitive changeset, particularly after a navigation from the scroll objective list: when you jump to changeset "X", you'd tend to expect "X" to be shown in the header, but the next changeset may be shown if "X" is too short.

Instead, select the changeset near the top of the screen (spanning an invisible line slightly below the banner).

Test Plan

Scrolled and jumped through a document with long and short changesets, saw a more intuitive changeset selected by the banner.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Here's an example:

Screen Shot 2017-05-20 at 4.30.34 AM.png (611×1 px, 108 KB)

The old code would have shown "changeset-view.css" in the banner. The new code shows "packages.php" in the banner, which I think aligns better with expectation.

This revision is now accepted and ready to land.May 20 2017, 1:52 PM
This revision was automatically updated to reflect the committed changes.