Redo the diff viewer column sizing logic to speed up render/resize times.
Review Request #8893 — Created April 10, 2017 and submitted
We have a certain sizing behavior for the columns in the diff viewer
that keeps them fairly equal but allows for some additional room if one
side or another has wider content. To achieve this, we needed to set
minimum and maximum widths on each of the <pre> tags, and we did so by
injecting a<style>
for each diff that we could control. This worked
well on most browsers, but hit performance problems on Firefox.We now have a different approach to this. We no longer inject a
<style>
into the document for each diff. Instead, we set this only for
the filename and revision columns (directly on the elements). We then
set styles to ensure the text is broken at the character level so that
it will always wrap if needed, for lines of any size.This does result in a behavior change. We formerly would break at the
word level, breaking on the character level if hitting the width
boundary, and wrapping text to the next line (to help with readability)
if possible. We no longer have that option (we could enable this just
for WebKit, which supports the non-standardword-break: break-word
, but
this creates an inconsistency that isn't worth having -- note that
word-wrap: break-word
does not exhibit the same behavior in our case).
The upside to breaking at the character level is that it's easier to see
spaces that might get lost in the wrapping. We'll see if people complain.
Tested that resizing logic worked correctly and that column sizing was
working the same way as it did with the old logic.Tested that Firefox no longer had the same slowdowns on larger diffs that
I was reproducing before.
Description | From | Last Updated |
---|---|---|
Mind including in this comment which (supported) browsers need it? |
david | |
Col: 13 'tbody' is defined but never used. |
reviewbot | |
Col: 13 'row' is defined but never used. |
reviewbot | |
Col: 13 'tbody_i' is defined but never used. |
reviewbot | |
Col: 13 'row_i' is defined but never used. |
reviewbot | |
Col: 13 '$paddingEls' is defined but never used. |
reviewbot | |
Col: 13 '$pre' is defined but never used. |
reviewbot |