Merge "Fix SideBySideCommentGroup mapping to only one peer on the other side"
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentGroup.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentGroup.java
index 20dd883..1d198ec 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentGroup.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentGroup.java
@@ -31,10 +31,11 @@
*/
abstract class CommentGroup extends Composite {
+ final DisplaySide side;
+ final int line;
+
private final CommentManager manager;
private final CodeMirror cm;
- private final DisplaySide side;
- private final int line;
private final FlowPanel comments;
private LineWidget lineWidget;
private Timer resizeTimer;
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideCommentGroup.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideCommentGroup.java
index 216fbda..ae6d3c1 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideCommentGroup.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideCommentGroup.java
@@ -21,6 +21,8 @@
import net.codemirror.lib.CodeMirror;
+import java.util.PriorityQueue;
+
/**
* LineWidget attached to a CodeMirror container.
*
@@ -28,14 +30,15 @@
* The group tracks all comment boxes on that same line, and also includes an
* empty padding element to keep subsequent lines vertically aligned.
*/
-class SideBySideCommentGroup extends CommentGroup {
+class SideBySideCommentGroup extends CommentGroup
+ implements Comparable<SideBySideCommentGroup> {
static void pair(SideBySideCommentGroup a, SideBySideCommentGroup b) {
- a.peer = b;
- b.peer = a;
+ a.peers.add(b);
+ b.peers.add(a);
}
private final Element padding;
- private SideBySideCommentGroup peer;
+ private final PriorityQueue<SideBySideCommentGroup> peers;
SideBySideCommentGroup(SideBySideCommentManager manager, CodeMirror cm, DisplaySide side,
int line) {
@@ -45,29 +48,42 @@
padding.setClassName(SideBySideTable.style.padding());
SideBySideChunkManager.focusOnClick(padding, cm.side());
getElement().appendChild(padding);
+ peers = new PriorityQueue<>();
}
SideBySideCommentGroup getPeer() {
- return peer;
+ return peers.peek();
}
@Override
void remove(DraftBox box) {
super.remove(box);
- if (0 < getBoxCount() || 0 < peer.getBoxCount()) {
- resize();
- } else {
+ if (getBoxCount() == 0 && peers.size() == 1
+ && peers.peek().peers.size() > 1) {
+ SideBySideCommentGroup peer = peers.peek();
+ peer.peers.remove(this);
detach();
- peer.detach();
+ if (peer.getBoxCount() == 0 && peer.peers.size() == 1
+ && peer.peers.peek().getBoxCount() == 0) {
+ peer.detach();
+ } else {
+ peer.resize();
+ }
+ } else {
+ resize();
}
}
@Override
void init(DiffTable parent) {
- if (getLineWidget() == null && peer.getLineWidget() == null) {
- this.attach(parent);
- peer.attach(parent);
+ if (getLineWidget() == null) {
+ attach(parent);
+ }
+ for (CommentGroup peer : peers) {
+ if (peer.getLineWidget() == null) {
+ peer.attach(parent);
+ }
}
}
@@ -76,20 +92,20 @@
getLineWidget().onRedraw(new Runnable() {
@Override
public void run() {
- if (canComputeHeight() && peer.canComputeHeight()) {
+ if (canComputeHeight() && peers.peek().canComputeHeight()) {
if (getResizeTimer() != null) {
getResizeTimer().cancel();
setResizeTimer(null);
}
- adjustPadding(SideBySideCommentGroup.this, peer);
+ adjustPadding(SideBySideCommentGroup.this, peers.peek());
} else if (getResizeTimer() == null) {
setResizeTimer(new Timer() {
@Override
public void run() {
- if (canComputeHeight() && peer.canComputeHeight()) {
+ if (canComputeHeight() && peers.peek().canComputeHeight()) {
cancel();
setResizeTimer(null);
- adjustPadding(SideBySideCommentGroup.this, peer);
+ adjustPadding(SideBySideCommentGroup.this, peers.peek());
}
}
});
@@ -102,7 +118,7 @@
@Override
void resize() {
if (getLineWidget() != null) {
- adjustPadding(this, peer);
+ adjustPadding(this, peers.peek());
}
}
@@ -117,6 +133,16 @@
private static void adjustPadding(SideBySideCommentGroup a, SideBySideCommentGroup b) {
int apx = a.computeHeight();
int bpx = b.computeHeight();
+ for (SideBySideCommentGroup otherPeer : a.peers) {
+ if (otherPeer != b) {
+ bpx += otherPeer.computeHeight();
+ }
+ }
+ for (SideBySideCommentGroup otherPeer : b.peers) {
+ if (otherPeer != a) {
+ apx += otherPeer.computeHeight();
+ }
+ }
int h = Math.max(apx, bpx);
a.padding.getStyle().setHeight(Math.max(0, h - apx), Unit.PX);
b.padding.getStyle().setHeight(Math.max(0, h - bpx), Unit.PX);
@@ -125,4 +151,14 @@
a.updateSelection();
b.updateSelection();
}
+
+ @Override
+ public int compareTo(SideBySideCommentGroup o) {
+ if (side == o.side) {
+ return line - o.line;
+ } else {
+ throw new IllegalStateException(
+ "Cannot compare SideBySideCommentGroup with different sides");
+ }
+ }
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideCommentManager.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideCommentManager.java
index a2af3a1c..2b83b71 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideCommentManager.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySideCommentManager.java
@@ -23,6 +23,7 @@
import net.codemirror.lib.TextMarker.FromTo;
import java.util.Collection;
+import java.util.Map;
import java.util.SortedMap;
/** Tracks comment widgets for {@link SideBySide}. */
@@ -94,36 +95,33 @@
@Override
CommentGroup group(DisplaySide side, int line) {
- SideBySideCommentGroup w = (SideBySideCommentGroup) map(side).get(line);
- if (w != null) {
- return w;
+ CommentGroup existing = map(side).get(line);
+ if (existing != null) {
+ return existing;
}
- int lineA;
- int lineB;
- if (line == 0) {
- lineA = lineB = 0;
- } else if (side == DisplaySide.A) {
- lineA = line;
- lineB = host.lineOnOther(side, line - 1).getLine() + 1;
+ SideBySideCommentGroup newGroup = newGroup(side, line);
+ Map<Integer, CommentGroup> map =
+ side == DisplaySide.A ? sideA : sideB;
+ Map<Integer, CommentGroup> otherMap =
+ side == DisplaySide.A ? sideB : sideA;
+ map.put(line, newGroup);
+ int otherLine = host.lineOnOther(side, line - 1).getLine() + 1;
+ existing = map(side.otherSide()).get(otherLine);
+ CommentGroup otherGroup;
+ if (existing != null) {
+ otherGroup = existing;
} else {
- lineA = host.lineOnOther(side, line - 1).getLine() + 1;
- lineB = line;
+ otherGroup = newGroup(side.otherSide(), otherLine);
+ otherMap.put(otherLine, otherGroup);
}
-
- SideBySideCommentGroup a = newGroup(DisplaySide.A, lineA);
- SideBySideCommentGroup b = newGroup(DisplaySide.B, lineB);
- SideBySideCommentGroup.pair(a, b);
-
- sideA.put(lineA, a);
- sideB.put(lineB, b);
+ SideBySideCommentGroup.pair(newGroup, (SideBySideCommentGroup) otherGroup);
if (isAttached()) {
- a.init(host.getDiffTable());
- b.handleRedraw();
+ newGroup.init(host.getDiffTable());
+ otherGroup.handleRedraw();
}
-
- return side == DisplaySide.A ? a : b;
+ return newGroup;
}
private SideBySideCommentGroup newGroup(DisplaySide side, int line) {
