Diffs in PolyGerrit apply two shades of highlight to changed lines
(light and dark) to indicate the granularity of modifications and to
distinguish intraline edits. However, the logic for choosing the
background shade for diff lines would differ from that of GWT UI diffs
subtly.
+----------------------------------+----------------------------------+
| GWT UI Shading Logic | PG Shading Logic (incorrect) |
+----------------------------------+----------------------------------+
| Diff lines get a dark background | Diff lines get a dark background |
| IFF they appear in a delta chunk | IFF they do NOT contain any |
| that is empty on the left OR | intraline differences. |
| empty on the right. | |
+----------------------------------+----------------------------------+
| Diff lines get a light background otherwise. |
+---------------------------------------------------------------------+
With this change, the shading logic in PolyGerrit is modified to match
that of the GWT UI.
Bug: Issue 4219
Bug: Issue 5117
Change-Id: Ice24292df777118c08c3e73f771720f8a186a183
Add a simple annotation layer that marks trailing whitespace in diffs
(guarded by the `show_whitespace_errors` diff preference). The newly
supported diff preference is added to both diff preference controls. The
requirement that all annotation layers must implement `addListener` is
relaxed as the trailing whitespace layer is the third layer that doesn't
use it.
Adds tests for the layer and the diff preference.
Feature: Issue 4836
Change-Id: Ifba05216bf0bc3c0a8a094f5ef392b983091d59f
Previously, the line marker was only subtly visible by the highlighted
line number. This change adds a bottom border to the selected line if
the user is using keycodes (j, k, up, down) to more the cursor. When
the escape key is pressed, the distinguished line marker will dissapear.
Feature: Issue 4739
Change-Id: If8c751efc137ef87cfdad1c8bf7d905de1219107
A css rule that was added for the full width diff caused lines to wrap
incorrectly. I've adjusted the style so that it is only present with
the full-width class.
Bug: Issue 4870
Change-Id: Ie94d0a06a125efb9fe7fa25ff7f2d45f54331d64
Previously in Polygerrit, diff views were always displayed in the width
specified in diff preferences. This change gives the option to wrap
lines instead, which takes precedence over column width (the column
width option is hidden when line wrapping is selected), and fits the
diff view to screen.
The gerrit API already supports the 'lineWrapping' preference so this
change uses that already existing option.
Feature: Issue 4809
Change-Id: I0d9e292739b5910abfd04af63ec4c745bf06e446
Diff header scrolls with content and sticks to the top.
Implemented custom gr-header wrapper to accomodate this.
Bug: Issue 4491
Change-Id: I451ecd4f6c454fd9ab3085ad8f9c5bdd27cb9269
This reverts commit a7b89441bda7a1ea7e98969462c2def989bd68c2.
Reason for revert: Got a better idea for app layout, will send follow-up change.
Change-Id: I7b7c86288fb070114e76dd68701e05c539089eab
This reverts commit 37ef1672f9133b605c5e14f8fbcc241d4c40eb50.
Reason for revert: Got a better idea for app layout, will send follow-up change.
Change-Id: I7bfddcef416d44c7af88d74da6b39c2a93565d32
Formerly, GPU rendering was hinted for diff tables on TBODY elements for
scrolling performance on large diffs. This would cause the right border
of the table appear inconsistently in Chrome -- likely as a fluke of
GPU layer compositing. With this change, the entire TABLE is hinted
rather than the TBODIES inside it, allowing them to be composited
together.
Bug: Issue 4801
Change-Id: I952a2d71a2ebd2d92b326b565b85fb55b52927cf
Fixes footer at the bottom of the page for all pages and changes
scrolling behavior for diff page.
On the diff page:
- horizontal scrolling is right above footer when required
- all headers stay in place when scrolling
Bug: Issue 4491
Change-Id: I62fdf130376b210c26fadcadc90bf452361ac15d
Because code comments are rendered using italics, and because italics
can sometimes draw text outside of their bounding boxes, some characters
may be visually obscured when they are adjacent to an annotation with a
background color -- particularly intraline difference annotations. With
this change, intraline difference background is given an alpha channel
so that such characters are not obscured.
Bug: Issue 4785
Change-Id: I5372e6f4def5e69126288965eb9829e5680c2102
This change adds a font size field in diff preferences in both the
generic preferences page and also the preferences modal from the diff
view. The new field allows users to update their preferred font size
for the diff view to be displayed. This updates both the line number
font size as well as the diff font size.
Feature: Issue 4417
Change-Id: I0fb75fd2186d2ff63b3627e8b0f15f2b3cddbfa3
This change uses a new method of generating the clipboard data that
circumvents several strange issues caused by shadow/shady DOM. This
new method is much more legible and abstracts more logic away from the
DOM.
Bug: Issue 4494
Change-Id: I8c186d6cbbe9536548d934f734856b1f9ced1a26
The addition of syntax highlighting silently broke copy/paste
functionality due to the addition of another layer of div nesting.
Related to this bug are some issues with correct text selection in
unified diff view, so this patch addresses them as well.
Bug: Issue 4317
Change-Id: Iac7379de4131ab4e44905a54218d42fcfe67ce62
This change reverts visible tabs to use the » character. A few different
approaches have been used for rendering these tab indicators:
I: Before the Annotation Pipeline, tab indicators were configured by a
class that was optionally applied to tab elements by the diff
builder. The class was guarded by the show_tabs preference and a CSS
rule created a `::before` pseudo element to insert the character.
(Thus also preventing the » from being copyable text.)
II: When the Annotation Pipeline was implemented, the ordering of layers
called for intraline difference elements being rendered *inside* tab
indicators. As a result, the » indicator would sometimes have a
different background than the intraline difference, looking sloppy.
To solve this, the pseudo element was positioned using absolute,
allowing the pseudo element to consume no horizontal space and and
the intraline background to extend across the entire tab.
III:When performance tuning, it was discovered that the
absolute-positioned tab indicators were positioned incorrectly when
GPU acceleration was hinted for the diff, so the containing tab
elements were made relative.
IV: Continuing performance tuning, the tab indicators seemed to slow
scrolling on very large diffs with tabs. It was mistakenly assumed
(by me) that this was related to the pseudo-elements when it was
actually caused by the thousands of positioning contexts they
created using relative and absolute.
Instead they were modified to use strike-through instead of a pseudo
element, which was more-performant, but less-usable (it looked bad).
With this change, we roll-back the clock a little and solve a core
problem: namely that tab indicators were not annotated inside the
intraline differences. Fixing that, positioning tricks are no-longer
needed to make the background look right.
To do this, we decouple the tab elements (which control tab width) from
the tab indicators (which optionally make tabs visible). This is done
using an annotation layer that inserts tab indicator elements wherever
a tab character is used in the source content, and it does so after
intraline differences have been applied.
Bug: Issue 4441
Change-Id: I4fea2a3a20a7039bfeb746bd1e1c1004e43c4801
A source of latency when creating diff comments in large diffs is the
work needed to reflow the diff DOM to make room for the new comment.
This is particularly evident when adding comments to new files because
the diff is built as an addition group representing the entire file, so
the comment causes a reflow on every subsequent line.
This change optimizes this process in three ways.
* **Limit the size of ADD & REMOVE groups:** The diff processor will now
break an add or a remove chunk into a series of smaller chunks of the
same kind. This is controlled by the MAX_GROUP_SIZE constant. In this
way the number of nodes that need to be reflowed when a comment is
added to an add or remove group is limited to the number of subsequent
lines in that group plus the subsequent number of groups.
* **GPU optimize group in general:** Adds CSS properties to diff TBODY
elements (which correspond to groups, for the most part) that trigger
GPU acceleration when available.
* **Apply `table-layout: fixed;`** This style speeds up table reflow in
general.
Change-Id: Ie0e3665b7752fec67f7123cfae70ae99e6f67521
Improves the speed of diff (particularly when scrolling in a large diff)
by removing the hover state for line numbers and by removing the pseudo
elements used in visible tabs.
Change-Id: I3c8687ec282696842b51f8f43b76aa267a1506b1
When a change contains removal of large block of text, in left diff side
it is highlighted with red, and in right side there's a corresponding
empty space. Hiding left diff side leaves gaps in the code.
This change hides those gaps when left diff side is hidden by Shift+A.
Feature: Issue 3926
Change-Id: I11c6574c098a9ecfe58d1804ba57c5c4f97e4d1a
Introduces the gr-syntax-layer element. This element works as an
annotation layer that is configured with the diff and asynchronously
computes/applies syntax for the diff.
Introduces a custom build of Highlight.js which gr-syntax-layer makes
use of. Building the script is documented in
scripts/vendor/highlight/building.md.
The layer is connected to the annotation pipeline in gr-diff-builder as
the lowest layer and syntax processing is triggered only after a diff
has been completely rendered.
A number of styles are added to the gr-diff element for syntax markers.
Tests added for gr-syntax-layer.
Bug: Issue 3916
Change-Id: Ic33e40f4fe39dfce1a62de133cfaf32be5e3f25a
Adds a polymorphic method to GrDiffBuilder subclasses named
`_getNextContentOnSide` which gets the a content element by traversing
from its preceding content on the same side. This method is dramatically
faster than the query-based method when diff sections are large.
In preparation for the syntax highlighting change, the gr-diff-builder
is refactored to use a property for the diff object, rather than
accepting it as a parameter to the `render` function.
Tests are included for new functions.
Change-Id: Ifd0edb530a303de2626d55a691c3ba1eaed6167c
Apply diff annotations (intraline differences and comment ranges) by
executing the annotation layers in order to each line. The diff builder
maintains an ordered array of annotation layers which are communicated
to GrDiffBuilder subclass instances. The builder also listens to each
layer for notifications that annotations have changed for some line
range and re-renders (i.e. re-applies the pipeline on DIV.contentText
elements) accordingly.
Change-Id: Iea0599d4869cafaadc0974158153a91d927913e8
The `_addIntralineHighlights` method of GrDiffBuilder is rewritten to
take advantage of GrAnnotate. In particular, formerly, the method would
apply intraline difference highlights by modifying an HTML string.
With this change, the highlight positions are translated as calls to
`GrAnnotate.annotateElement`, which works on the `DIV.contentText`
element.
Change-Id: I2838ef29f057f1108e2ffd196bb48a239366dc87
This reverts commit 56689af0f92ce13f90ff9369544c5f9cc0412f09.
Reason for revert: This change broke adding draft comments in the diff
view.
Change-Id: Icfbd3eb4e24cce3e1690e7eaf12e14e5705c7e3e
Adds two buttons to appear in context control lines in diffs. Whereas
the main button in a context control replaces the control with *all* of
the diff content that was collapsed into it, the new buttons will
instead display 10 lines of diff content either at the start or at the
end of the collapsed area.
If the number of lines collapsed into the context control are fewer than
11, the +10 buttons are not displayed.
Bug: Issue 3942
Change-Id: I03d94d8f1c0aca626e9cec9b63961c5a3e9e0759
If gr-diff recognizes that the file difference it's representing is
between images, it uses a different diff-builder that displays images
in a side-by-side-manner. In this case gr-diff will also make requests
for the image data itself, which it can pass down into the image-based
diff-builder.
Adds methods to gr-rest-api-interface to support rendering the data
relevant to image diffs. For images that are revisions of the current
change, provides "getChangeFileContents". For images that come from the
parent tree (i.e. if the basePatchNum is "PARENT") the interface
provides "getCommitInfo" to determine the SHA of the parent commit, and
"getCommitFileContents" which can get file contents for a given commit.
Bug: Issue 3822
Change-Id: I9be025b4e549fca97c87cdbeede6cb64dea5eac0
Reorganizes the keyboard shortcut list for change view with smaller
groups of shortcuts. Also tidies up the dialog for the diff view a bit.
Updates the style for the selected line in the diff view and fixes the
focus when the diff expand/collapse buttons are activated. The scroll
behavior is changed slightly.
Change-Id: I8e520f725f4706aaad6e8bd8b99dd0e274d2f830
Adds gr-diff-cursor, which is a specialized cursor for navigating
through diff content. It tracks the diff line being targeted, and, in
side-by-side mode, it tracks the side of the diff being targeted.
Also includes special behavior that ships blank spaces and other
non-commentable content in diffs, as well as automatically switching
sides when appropriate.
Support for more than one diff.
Also updates the diff builders to emit some useful element classes.
Note that this only adds the diff cursor along with tests. This is
setting up the work to actually add the diff cursor to the diff views.
Bug: Issue 4033
Change-Id: I991fd514f56ddd19d43d8e1354ad0d4fc71930c4
The gr-diff element was already capable of rendering diffs in either
side-by-side or unified style, but was configured by default to use
side-by-side. The GWT diff UI included an icon button toggle to switch
between these two styles above and to the right of the diff near the
"Diff View Preferences" button. This change introduces a selector in the
PolyGerrit UI to allow similar switching behavior.
Previously, the diff mode was encoded as a private property of the
gr-diff element. Now that the switcher is introduced at the same level
as the "Diff View Preferences" button (one level up from the gr-diff
element) it is changed to be a public property of the element so that it
can be changed through the parent view (in this case the gr-diff-view
element).
A dropdown is added to the gr-diff-view based in style on the
gr-patch-range-select element (which plays a similar role). This
dropdown has options for either of the two diff modes and controls the
diff display through a property on the view.
The view gets a new, computed property called _diffMode which encodes
which diff state should be displayed based on the changeViewState and
the user's preferences. The user's choice of diff view is persisted
across diff views, but is reverted to the preference when the change
view is visited.
A test is added to the gr-diff-view element.
Bug: Issue 3909
Change-Id: I00d93500f8d210394acc9c8f3398c9406ae74276
This will allow for an easier time in implementing expanding
inline diffs by having gr-diff only responsible for rendering the
diff itself and not other controls.
Change-Id: I254ad5900091c731e2197590d6043c103216785e
For table cells, setting the width css attribute isn’t sufficient to
prevent them from shrinking. Set min and max width.
Change-Id: I02592a8e41f55a9f250cee2735c4cc7ec9566e09
+ Cleans up interface for grouping comments within
gr-rest-api-interface.
+ Removes gr-diff’s dep on gr-ajax and rest-client-behavior.
Change-Id: Idea54a38fa5d0b136ff00b7ec7322bd24ad5d1bd
