75 Commits

Author SHA1 Message Date
Viktar Donich
cc6b8300d1 Revert "Fix app size to make scrollbars visible"
This reverts commit a7b89441bda7a1ea7e98969462c2def989bd68c2.

Reason for revert: Got a better idea for app layout, will send follow-up change.

Change-Id: I7b7c86288fb070114e76dd68701e05c539089eab
2016-10-24 20:53:30 +00:00
Viktar Donich
a7b89441bd Fix app size to make scrollbars visible
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
2016-10-20 12:09:57 -07:00
Wyatt Allen
3894bb9f2a Use semi-transparent intraline highlight color
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
2016-10-18 18:18:00 -07:00
Andrew Bonventre
fd434afe59 First round cleanup of network requests & errors from tests
+ These were slowing down tests in cases where it would actually hit a
  live server, potentially adding the latency from the network to the
  test.
+ Other fixes involve removing unused imports of util.js amongst other
  small tweaks/fixes.

Bug: Issue 4016
Change-Id: I442deefebeffc6a701e4922faccfe1c74b3a35b6
2016-10-16 06:56:37 +00:00
Becky Siegel
1f5a8510b7 Safeguard for font preferences if server has skew
Added temporary safeguard for the font preferences change so that inputs
are hidden and custom styles are not set if font-size was not part of
the preferneces response.

Change-Id: I5b34b317bf953591889fa25e1c351d4f490d989e
2016-10-14 14:13:36 -07:00
Andrew Bonventre
0c03db348c Merge "Add a diff preference for font size" 2016-10-14 17:53:05 +00:00
Wyatt Allen
1be7c31e66 Gracefully handle HTTP 409 for diff API
If a file is too large for the server to deliver the diff, the API will
respond with HTTP 409. PolyGerrit interprets th this as a network error
and covers the entire page with an error message. Furthermore, the
handler for loading the diff is not written for this kind of failure and
tries to dereference the diff object -- null in that case.

With this change, the 409 message does not block use of the whole page
and the handler does not break when the diff is null.

Issue: Bug 4770
Change-Id: I68fe50c474fc03d4217e969649f62df38ca5b632
2016-10-13 11:50:14 -07:00
Becky Siegel
d7dbed640c Add a diff preference for font size
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
2016-10-12 16:31:24 -07:00
Wyatt Allen
056e40c01c Fix diff expanded/collapsed binding
Refactors bindings in file lists to change the expanded state of a diff
using a property name that does not collide with the hidden attribute.

Change-Id: I7f7e38a910a8d4dd19be2d591033be6f6d3cb7a1
2016-10-05 21:49:33 -07:00
Kasper Nilsson
bf457dd412 Fix data binding in gr-file-list, introduced in 86117
In https://gerrit-review.googlesource.com/c/86117/ the data binding
between the _files array and the dom-repeat in gr-file-list was not
properly enforced. This change fixes that, and also properly
references a static file-list property.

In addition, tests were implemented to ensure this regression is caught
in the future.

Bug: Issue 4575
Change-Id: I32a5645ec47d953ee6a7a71f4b14fc8c9483df3d
2016-09-30 13:04:15 -07:00
Wyatt Allen
9fcec7437f Generate clipboard data via diff rather than DOM reconstruction
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
2016-09-13 14:24:57 -07:00
Andrew Bonventre
cca7fb3df1 Merge "Fix copy/paste in diff view" 2016-09-01 23:37:17 +00:00
Kasper Nilsson
613b49c64e Fix copy/paste in diff view
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
2016-08-29 16:18:47 -07:00
Kasper Nilsson
93b0ad4964 Add expand inline functionality to diffs
This commit enables the user to manually expand individual diffs
inline in the file list of the change view.

Bug: Issue 4382
Change-Id: I87d5af9971fed3aa5e1eb64523f4623a5ff2ac8d
2016-08-26 10:36:30 -07:00
Wyatt Allen
be0142ca03 Improves visible tabs rendering
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
2016-08-25 11:47:17 -07:00
Andrew Bonventre
87d7d86a3d Merge "Applies optimizations to diff comment DOM attachment" 2016-08-11 20:30:20 +00:00
Wyatt Allen
32b03fc76f Applies optimizations to diff comment DOM attachment
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
2016-08-11 12:35:31 -07:00
Andrew Bonventre
a51f78b1b2 Merge "Reduces the cost of Diff Reflow" 2016-08-11 18:06:18 +00:00
Wyatt Allen
a04b5c1ae6 Reduces the cost of Diff Reflow
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
2016-08-11 10:55:02 -07:00
Viktar Donich
7706e9cff9 Hide gaps from right diff when hiding left diff
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
2016-08-10 14:55:22 -07:00
Viktar Donich
c282d7b282 Toggle left diff on Shift+A
Feature: Issue 3926
Change-Id: Ia52334f2aefeae4b3f115e5779da3b61af25ba11
2016-08-10 12:54:49 -07:00
Wyatt Allen
9f7e37cbf7 Fixes an if-else chain
Change-Id: I5181644364a959461a4ad5fdc23e24a57ffe5052
2016-08-08 14:36:24 -07:00
Viktar Donich
9187269b56 Prevent empty diff content lines from collapsing
Bug: Issue 4330
Change-Id: I506be8ab6c9cc0e14c41abd78e87736da9f470a1
2016-08-05 16:28:00 -07:00
Viktar Donich
b34d1f87c7 Show file weblinks (e.g. gitles) in diff view
Bug: Issue 4205
Change-Id: Ib5c832bb29453aa51ebe73732c7b8e6885e7e12f
2016-08-03 14:52:13 -07:00
Wyatt Allen
ec4d6a28a4 Modularize syntax theme
Change-Id: I682bdbeb6b8bd4dbb4b389c52043db4b6c19286e
2016-07-26 10:38:15 -07:00
Wyatt Allen
650c529276 Syntax highlighting
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
2016-07-25 13:47:30 -07:00
Wyatt Allen
ea97c5b49a Adds diff traversal helper and make diff object a builder property
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
2016-07-21 22:26:56 -07:00
Wyatt Allen
d0dd392794 Establish annotation pipeline
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
2016-07-20 12:25:40 -07:00
Urs Wolfer
33df005810 Fix issues detected by 'JSHint' and 'JSCS'
Change-Id: Ic1437333fcf82473ac57f8bdea25ee8188ddbfee
2016-07-15 20:32:27 +02:00
Viktar Donich
60d8bce771 Release range comments
Remove beta diff setting and enable range comments for all.

Change-Id: I0cee7e1717574ab54317cdce9e2332c8cda7a0cb
2016-07-14 14:45:15 -07:00
Wyatt Allen
b7fa5dd929 Applies intraline differences as annotations
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
2016-07-13 21:07:13 -07:00
Wyatt Allen
025e65d7d5 Reorganized DOM for diff content in PolyGerrit
Formerly, diff content elements mixed text with comment threads. For
example, a diff content node with an intraline highlight, a ranged
comment, and a gr-diff-comment-thread may have been organized as below:

    TD.content
      ╠ #text
      ╠ HL (intraline difference)
      ║ ╚ #text
      ╠ #text
      ╠ HL.range (ranged comment highlight)
      ║ ╚ #text
      ╠ #text
      ╚ GR-DIFF-COMMENT-THREAD
        ╠ GR-DIFF-COMMENT
        ╚ ...

Note that the comment thread was inserted at the same level as the text
of the diff line.

With this change, the text is separated from the comment thread by
introducing a DIV to contain the text with class `contentText` as
sibling to comment threads.

    TD.content
      ╠ DIV.contentText
      ║ ╠ #text
      ║ ╠ HL
      ║ ║ ╚ #text
      ║ ╠ #text
      ║ ╠ HL.range
      ║ ║ ╚ #text
      ║ ╚ #text
      ╚ GR-DIFF-COMMENT-THREAD
        ╠ GR-DIFF-COMMENT
        ╚ ...

Modifies the `getContentByLine` method of gr-diff-builder to return the
`DIV.contentText` element rather than the `TD.content` element which is
its parent. In most uses of this function, the text is what is needed
rather than the TD or comment thread, but in other cases, they can be
easily DOM traversed.

Change-Id: I0eded34afd3d22963252efc7eabfee290ae21a9c
2016-07-11 11:20:09 -07:00
Wyatt Allen
7bbc6b49c0 Add line range information to group objects
Group objects now keep track of the range of line numbers for the lines
they contain, making specific groups easier to identify.

Change-Id: I37873d83e1003d75df7da77e619e23208d1c30b3
2016-07-08 12:50:48 -07:00
Wyatt Allen
db881fa0ad Asynchronous diff rendering
Building on existing support for asynchronous diff processing, the
rendering stage is now also asynchronous. The `emitGroup` method of
gr-diff-builder (which constructs a DOM fragment and attaches it to the
document) is now called whenever the processor emits a new group, rather
than waiting for all groups to be available and looping over them.

Adds support for canceling the processor, and removes a behavior where
the diff was being re-rendered needlessly, causing visible flicker.
Updates a test that broke when rendering became asynchronous.

Change-Id: I37fcd65efc8c901b85fc93e91611c022edc10dc4
2016-06-30 10:09:19 -07:00
Wyatt Allen
16810999f6 Prevent redefinition of class libraries
A number of classes used in PolyGerrit are defined in vanilla JS files
that are included multiple times by various elements.  For example,
gr-diff-line.js is included by the gr-diff-builder and by gr-diff
elements. Adds #ifndef-style guards to each of these libraries to
prevent redefinition and avoid different elements potentially referring
to different versions of the same class.

Change-Id: I45e3ba425a59989b328475b1fe58fd9f350c8ae0
2016-06-27 09:43:06 -07:00
Wyatt Allen
66bfe0005d Move the diff cursor when the diff content is tapped
When a user taps a line number in a diff, the cursor is moved to that
line and that side of the diff. However, if the user taps the text
content of a diff, the cursor was not moved. With this change, the
cursor is moved to the appropriate line and side of a diff when either
is tapped.

Bug: Issue 4215
Change-Id: I47d24f678f487eb3f8173ea5572865a589d845e4
2016-06-21 13:58:22 -07:00
Wyatt Allen
bdd4e3498c Move the diff cursor to line number specified by the URL hash
If the gr-diff-view is loaded with a line number address in the URL
hash, moves the diff cursor to that line.

Bug: Issue 4206
Change-Id: I359a29d7a3fd97b0ce72228aec3f49ae2295bfee
2016-06-21 08:47:36 -07:00
Wyatt Allen
a68b48e1f5 Update the URL hash with the selected diff line
Sets the URL hash to the address of the cursor when a user selects a
diff line (either by clicking the line number or using the 'c' hotkey).
If the cursor is on a line of the revision, the address is the line
number. If the cursor is on a line of the base patch, the address is the
letter 'b' followed by the line number. Otherwise the address is the
empty string.

Bug: Issue 4206
Change-Id: Ic85da197eb6264ea1111cc34e781dccbc24d4d40
2016-06-20 14:14:55 -07:00
Logan Hanks
1344623fc1 Add missing property declarations to gr-diff
Change-Id: I68378000129fefa51d9cf30ee8b483ee8e06be1b
2016-06-15 14:27:30 -07:00
Viktar Donich
b2198e8233 Ranged comments integration
- gr-file-list recognizes local preferences (for hasRangedComments flag)
- gr-file-list reacts to cursor hotkey only if there is no range
  selected (currently always false).
- Remove dead code from GrDiffBuilderSideBySide, GrDiffBuilder,
  gr-diff-builder.html
- Bugfix: GrDiffBuilder.prototype.getGroupsByLineRange handles one-line
  BOTH code sections correctly. Test updated as well.
- Added utitily methods added to gr-diff-builder.html to reduce
  dependency on DOM structure and reduce amount of code copy-pasting:
  - renderLineRange, getContentByLine, etc
- For gr-diff.js and gr-diff-comment-thread.js addDraft renamed to
  addOrEditDraft because that's what it does.
- For both, addDraft method always creates a draft comment.
- Added support for ranged comments in gr-diff, gr-diff-comment-thread.
- Added mouseenter and mouseout events to gr-comment.js
- Refactored gr-comment.js to reduce code copy-paste, unify event
  payload, and to eliminate need of accessing component instance for
  patchNum. Tests updated as well.
- Refactored gr-diff.js UI data model update using gr-diff-builder.html
  utility methods to make code more readable.
- Added support for creating ranged comments to gr-diff.js.
- gr-selection-action-box now reacts to click and tap to create a 
  comment.

Change-Id: I01480a4c6f460774a8b2826915702800b3f81d25
2016-06-13 22:44:47 +00:00
Viktar Donich
ab1b211d80 Ranged highlights: initial commit
- Initial commit: structure, 'enabled' property controls sample
mouseDown handler.
- Minimal integration: isRangeSelected
- Minimal test.

Change-Id: I963ac7b158d57a1c24ee302d99eb6af26b0aeeeb
2016-06-13 14:38:08 -07:00
Viktar Donich
e1341972d1 Move text selection out of gr-diff.
Change-Id: I0734653066a1bb78f95c141aa8202fad315b13c0
2016-06-10 12:18:25 -07:00
Viktar Donich
5008b4491f Add a checkbox to hide ranged comments behind
Also adds concept of locally stored preferences.

Change-Id: Ib074a682228d5360a932af696e18967e8e3473be
2016-06-08 15:29:11 -07:00
Viktar Donich
0f02cda57e Make gr-builder a Polymer component
Updated tests, fixed draft comments, context expanding.

Change-Id: Ic4bd9682c63edd8e80fbc2abcb4fa5e406a202ab
2016-06-03 10:22:47 -07:00
Andrew Bonventre
6c9d862e10 Revert "Make gr-builder a Polymer component"
This reverts commit 56689af0f92ce13f90ff9369544c5f9cc0412f09.

Reason for revert: This change broke adding draft comments in the diff
view.

Change-Id: Icfbd3eb4e24cce3e1690e7eaf12e14e5705c7e3e
2016-06-03 13:07:33 +00:00
Viktar Donich
56689af0f9 Make gr-builder a Polymer component
Change-Id: I0d99775332b0ef473459ad481970ad6033427c7d
2016-06-02 13:09:39 -07:00
Wyatt Allen
1097963107 Adds "+10 lines" buttons to diff context controls
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
2016-06-01 16:25:20 -07:00
Viktar Donich
679fa309c2 Gr-diff retrofit: diff section state.
Fixing UI data pipe line, re-rendering:
 - store section DOM Elements in gr-diff-builder
 - find diff section by line range
 - store collapsed lines as gr-diff-group
 - update gr-diff-builder's data model on expanding collapsed lines
 - .right/.left CSS classes applied to td.lineNum instead of .content

Feature: Issue 3910
Change-Id: Ia0cb78aa00cfd910d186a37631fe71a89204a843
2016-05-27 18:35:43 +00:00
Viktar Donich
7ad28920e9 Gr-diff retrofit: store comment state
Fixing UI data pipe line, re-rendering:
 - side, draft text and editing status in UI comment objects
 - update gr-diff UI model on comment save/update

Feature: Issue 3910
Change-Id: I96f714c7de9add6e316dcf64bb7d566690b9d3ae
2016-05-27 10:46:54 -07:00
Viktar Donich
b7c2cce46a Gr-diff retrofit: implement re-render.
Feature: Issue 3910
Change-Id: Ia85bb2302f8435fd6066873ce87bc9c08b9c63b1
2016-05-26 14:57:18 -07:00