Mostly stop page jumps when resolving or reopening issues.

Review Request #8906 — Created April 18, 2017 and submitted

Information

Review Board
release-2.5.x
c3f0c55...

Reviewers

When clicking the Fixed, Discard, or Reopen button, the updating of the
issue summary table can cause the page's scroll position to change. We
had code in place that intended to address this, but this was very
dependent on the order in which the browser updates the pages and
calculates positions. The old code assumed it could safely get the old
scroll position after triggering DOM updates, but this wasn't a good
assumption and no longer works in Chrome or Firefox.

We now fetch this value before performing any DOM updates, and we also
now set the scroll position in an animation frame. This is done in an
attempt to more closely tie the scroll position update with the DOM
update. While not necessary on Chrome or Firefox, it does reduce the
chances quite a bit of jumping on Safari and Internet Explorer (though
it will happen at times).

There is a very slight jump that's still noticeable in Firefox, but it's
actually unrelated to this logic. There are some heights/positions in
the review request box that are using subpixels, which affect rendering
as the issue summary table changes height. Those will be dealt with
separately.

Tested in Chrome, Firefox, Safari, and Edge.

Chrome works the best. There's absolutely no jump or perceived shift in
rendering when clicking any issue buttons now.

Firefox works mostly well, but as the issue summary table changes height,
subpixels elsewhere in the UI affect rendering. This is noticed as a slight
shift in horizontal lines, with the scroll position staying exactly the
same on every click.

Safari mostly stays in place when clicking buttons, but does sometimes have
a quick jump. I have not been able to eliminate this entirely, but
requestAnimationFrame reduces the occurrence.

Internet Explorer has both the sub-pixel bug and the occasional jump (again,
reduced by requestAnimationFrame, but otherwise mostly stays in place.

david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.5.x (c1e5023)
Loading...