opendev/gerrit
Code Issues Proposed changes

Merge "Click icon to add file comment in SideBySide view."

Browse Source
This commit is contained in:
Martin Fick 2012-11-16 19:36:07 -08:00 committed by Gerrit Code Review
commit 9131687ea2
12 changed files with 142 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
gerrit-common/src/main/java/com/google/gerrit/common/data/CommentDetail.java
View File

@ -89,9 +89,6 @@ public class CommentDetail {
}
public List<PatchLineComment> getForA(final int lineNbr) {
if (lineNbr == 0) {
return Collections.emptyList();
}
if (forA == null) {
forA = index(a);
}
@ -99,9 +96,6 @@ public class CommentDetail {
}
public List<PatchLineComment> getForB(final int lineNbr) {
if (lineNbr == 0) {
return Collections.emptyList();
}
if (forB == null) {
forB = index(b);
}

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritCss.java
View File

@ -56,6 +56,7 @@ public interface GerritCss extends CssResource {
String changeTypeCell();
String changeid();
String closedstate();
String cellsNextToFileComment();
String commentedActionDialog();
String commentedActionMessage();
String commentCell();
@ -134,6 +135,7 @@ public interface GerritCss extends CssResource {
String header();
String hyperlink();
String iconCell();
String iconCellOfFileCommentRow();
String iconHeader();
String identityUntrustedExternalId();
String infoBlock();

3
gerrit-gwtui/src/main/java/com/google/gerrit/client/GerritResources.java
View File

@ -48,4 +48,7 @@ public interface GerritResources extends ClientBundle {
@Source("queryProjectLink.png")
public ImageResource queryProjectLink();
@Source("addFileComment.png")
public ImageResource addFileComment();
}

BIN
gerrit-gwtui/src/main/java/com/google/gerrit/client/addFileComment.png Normal file
View File

Binary file not shown.
Side by Side

After

Width:  |  Height:  |  Size: 642 B

9
gerrit-gwtui/src/main/java/com/google/gerrit/client/gerrit.css
View File

@ -640,6 +640,9 @@
.patchContentTable tr.commentHolder .iconCell {
background: white;
}
.patchContentTable tr.commentHolder .iconCellOfFileCommentRow {
background: trimColor;
}
.patchContentTable td.commentHolder {
padding-left: 0;
padding-right: 0;
@ -763,6 +766,11 @@
text-decoration: underline;
}
.patchContentTable td.cellsNextToFileComment {
background: trimColor;
border-top: trimColor;
border-bottom: trimColor;
}
.patchContentTable .activeRow .iconCell,
.patchContentTable .activeRow .lineNumber {
background: selectionColor;
@ -775,6 +783,7 @@
.patchContentTable .activeRow .wdc,
.patchContentTable .activeRow .wdd,
.patchContentTable .activeRow .wdi,
.patchContentTable .activeRow .iconCellOfFileCommentRow,
.patchContentTable .activeRow td.commentHolder.commentPanelLast {
border-bottom: 1px solid blue;
}

40
gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/AbstractPatchContentTable.java
View File

@ -46,10 +46,12 @@ import com.google.gwt.event.dom.client.KeyPressEvent;
import com.google.gwt.event.shared.HandlerRegistration;
import com.google.gwt.user.client.DOM;
import com.google.gwt.user.client.Element;
import com.google.gwt.user.client.Event;
import com.google.gwt.user.client.History;
import com.google.gwt.user.client.ui.Button;
import com.google.gwt.user.client.ui.Focusable;
import com.google.gwt.user.client.ui.HTMLTable.CellFormatter;
import com.google.gwt.user.client.ui.Image;
import com.google.gwt.user.client.ui.UIObject;
import com.google.gwt.user.client.ui.Widget;
import com.google.gwtexpui.globalkey.client.GlobalKey;
@ -62,6 +64,8 @@ import java.util.List;
public abstract class AbstractPatchContentTable extends NavigationTable<Object>
implements CommentEditorContainer, FocusHandler, BlurHandler {
public static final int R_HEAD = 0;
static final short FILE_SIDE_A = (short) 0;
static final short FILE_SIDE_B = (short) 1;
protected PatchTable fileList;
protected AccountInfoCache accountCache = AccountInfoCache.empty();
protected Patch.Key patchKey;
@ -70,6 +74,8 @@ public abstract class AbstractPatchContentTable extends NavigationTable<Object>
protected boolean onlyOneHunk;
protected PatchSetSelectBox headerSideA;
protected PatchSetSelectBox headerSideB;
protected Image iconA;
protected Image iconB;
private final KeyCommandSet keysComment;
private HandlerRegistration regComment;
@ -108,6 +114,10 @@ public abstract class AbstractPatchContentTable extends NavigationTable<Object>
table.setStyleName(Gerrit.RESOURCES.css().patchContentTable());
}
abstract void createFileCommentEditorOnSideA();
abstract void createFileCommentEditorOnSideB();
abstract PatchScreen.Type getPatchScreenType();
protected void initHeaders(PatchScript script, PatchSetDetail detail) {
@ -116,6 +126,23 @@ public abstract class AbstractPatchContentTable extends NavigationTable<Object>
headerSideB = new PatchSetSelectBox(PatchSetSelectBox.Side.B, type);
headerSideA.display(detail, script, patchKey, idSideA, idSideB);
headerSideB.display(detail, script, patchKey, idSideA, idSideB);
iconA = new Image(Gerrit.RESOURCES.addFileComment());
iconA.setTitle(PatchUtil.C.addFileCommentToolTip());
iconA.addClickHandler(new ClickHandler() {
@Override
public void onClick(ClickEvent event) {
createFileCommentEditorOnSideA();
}
});
iconB = new Image(Gerrit.RESOURCES.addFileComment());
iconB.setTitle(PatchUtil.C.addFileCommentToolTip());
iconB.addClickHandler(new ClickHandler() {
@Override
public void onClick(ClickEvent event) {
createFileCommentEditorOnSideB();
}
});
}
@Override
@ -382,7 +409,7 @@ public abstract class AbstractPatchContentTable extends NavigationTable<Object>
protected void createCommentEditor(final int suggestRow, final int column,
final int line, final short file) {
if (Gerrit.isSignedIn()) {
if (1 <= line) {
if (R_HEAD <= line) {
final Patch.Key parentKey;
final short side;
switch (file) {
@ -417,9 +444,10 @@ public abstract class AbstractPatchContentTable extends NavigationTable<Object>
}
}
abstract void insertFileCommentRow(final int row);
private CommentEditorPanel findOrCreateCommentEditor(final int suggestRow,
final int column, final PatchLineComment newComment,
final boolean create) {
final int column, final PatchLineComment newComment, final boolean create) {
int row = suggestRow;
int spans[] = new int[column + 1];
FIND_ROW: while (row < table.getRowCount()) {
@ -477,7 +505,11 @@ public abstract class AbstractPatchContentTable extends NavigationTable<Object>
}
}
if (needInsert || !isCommentRow) {
insertRow(row);
if (newComment.getLine() == R_HEAD) {
insertFileCommentRow(row);
} else {
insertRow(row);
}
styleCommentRow(row);
}
table.setWidget(row, column, ed);

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchConstants.java
View File

@ -67,6 +67,7 @@ public interface PatchConstants extends Constants {
String reviewedAnd();
String next();
String download();
String addFileCommentToolTip();
String buttonReplyDone();
String cannedReplyDone();

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchConstants.properties
View File

@ -49,6 +49,7 @@ nextFileHelp = Next file
reviewedAnd = Reviewed &
next = next
download = Download
addFileCommentToolTip = Click to add file comment
fileTypeSymlink = Type: Symbolic Link
fileTypeGitlink = Type: Git Commit in Subproject

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchSetSelectBox.java
View File

@ -163,7 +163,7 @@ public class PatchSetSelectBox extends Composite {
return anchor;
}
private boolean isFile() {
public boolean isFile() {
boolean isCommitMessage = Patch.COMMIT_MSG.equals(script.getNewName());
return !(isCommitMessage || //
(side == Side.A && 0 >= script.getA().size()) || //

101
gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/SideBySideTable.java
View File

@ -40,7 +40,6 @@ import org.eclipse.jgit.diff.Edit;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
public class SideBySideTable extends AbstractPatchContentTable {
private static final int A = 2;
@ -50,6 +49,16 @@ public class SideBySideTable extends AbstractPatchContentTable {
private SparseHtmlFile a;
private SparseHtmlFile b;
protected void createFileCommentEditorOnSideA() {
createCommentEditor(R_HEAD + 1, A, R_HEAD, FILE_SIDE_A);
return;
}
protected void createFileCommentEditorOnSideB() {
createCommentEditor(R_HEAD + 1, B, R_HEAD, FILE_SIDE_B);
return;
}
@Override
protected void onCellDoubleClick(final int row, int column) {
if (column > C_ARROW && getRowItem(row) instanceof PatchLine) {
@ -159,9 +168,9 @@ public class SideBySideTable extends AbstractPatchContentTable {
if (del && ins) {
lines.add(new PatchLine(REPLACE, hunk.getCurA(), hunk.getCurB()));
} else if (del) {
lines.add(new PatchLine(DELETE, hunk.getCurA(), 0));
lines.add(new PatchLine(DELETE, hunk.getCurA(), -1));
} else if (ins) {
lines.add(new PatchLine(INSERT, 0, hunk.getCurB()));
lines.add(new PatchLine(INSERT, -1, hunk.getCurB()));
}
}
}
@ -189,6 +198,14 @@ public class SideBySideTable extends AbstractPatchContentTable {
initHeaders(script, detail);
table.setWidget(R_HEAD, A, headerSideA);
table.setWidget(R_HEAD, B, headerSideB);
// Populate icons to lineNumber column header.
if (headerSideA.isFile()) {
table.setWidget(R_HEAD, A - 1, iconA);
}
if (headerSideB.isFile()) {
table.setWidget(R_HEAD, B + 1, iconB);
}
}
private void appendModeLine(final SafeHtmlBuilder nc, final FileMode mode) {
@ -227,35 +244,41 @@ public class SideBySideTable extends AbstractPatchContentTable {
setAccountInfoCache(cd.getAccounts());
for (int row = 0; row < table.getRowCount();) {
if (getRowItem(row) instanceof PatchLine) {
final Iterator<PatchLineComment> ai;
final Iterator<PatchLineComment> bi;
if (row == R_HEAD) {
ai = cd.getForA(R_HEAD).iterator();
bi = cd.getForB(R_HEAD).iterator();
} else if (getRowItem(row) instanceof PatchLine) {
final PatchLine pLine = (PatchLine) getRowItem(row);
final List<PatchLineComment> fora = cd.getForA(pLine.getLineA());
final List<PatchLineComment> forb = cd.getForB(pLine.getLineB());
row++;
final Iterator<PatchLineComment> ai = fora.iterator();
final Iterator<PatchLineComment> bi = forb.iterator();
while (ai.hasNext() && bi.hasNext()) {
final PatchLineComment ac = ai.next();
final PatchLineComment bc = bi.next();
insertRow(row);
bindComment(row, A, ac, !ai.hasNext(), expandComments);
bindComment(row, B, bc, !bi.hasNext(), expandComments);
row++;
}
row = finish(ai, row, A, expandComments);
row = finish(bi, row, B, expandComments);
ai = cd.getForA(pLine.getLineA()).iterator();
bi = cd.getForB(pLine.getLineB()).iterator();
} else {
row++;
continue;
}
row++;
while (ai.hasNext() && bi.hasNext()) {
final PatchLineComment ac = ai.next();
final PatchLineComment bc = bi.next();
if (ac.getLine() == R_HEAD) {
insertFileCommentRow(row);
} else {
insertRow(row);
}
bindComment(row, A, ac, !ai.hasNext(), expandComments);
bindComment(row, B, bc, !bi.hasNext(), expandComments);
row++;
}
row = finish(ai, row, A, expandComments);
row = finish(bi, row, B, expandComments);
}
}
@Override
protected void insertRow(final int row) {
super.insertRow(row);
final CellFormatter fmt = table.getCellFormatter();
private void defaultStyle(final int row, final CellFormatter fmt) {
fmt.addStyleName(row, A - 1, Gerrit.RESOURCES.css().lineNumber());
fmt.addStyleName(row, A, Gerrit.RESOURCES.css().diffText());
fmt.addStyleName(row, B, Gerrit.RESOURCES.css().diffText());
@ -263,10 +286,36 @@ public class SideBySideTable extends AbstractPatchContentTable {
fmt.addStyleName(row, B + 1, Gerrit.RESOURCES.css().rightmost());
}
@Override
protected void insertRow(final int row) {
super.insertRow(row);
final CellFormatter fmt = table.getCellFormatter();
defaultStyle(row, fmt);
}
@Override
protected void insertFileCommentRow(final int row) {
table.insertRow(row);
final CellFormatter fmt = table.getCellFormatter();
fmt.addStyleName(row, C_ARROW, Gerrit.RESOURCES.css().iconCellOfFileCommentRow());
defaultStyle(row, fmt);
fmt.addStyleName(row, C_ARROW, //
Gerrit.RESOURCES.css().cellsNextToFileComment());
fmt.addStyleName(row, A - 1, //
Gerrit.RESOURCES.css().cellsNextToFileComment());
fmt.addStyleName(row, B + 1, //
Gerrit.RESOURCES.css().cellsNextToFileComment());
}
private int finish(final Iterator<PatchLineComment> i, int row, final int col, boolean expandComment) {
while (i.hasNext()) {
final PatchLineComment c = i.next();
insertRow(row);
if (c.getLine() == R_HEAD) {
insertFileCommentRow(row);
} else {
insertRow(row);
}
bindComment(row, col, c, !i.hasNext(), expandComment);
row++;
}

12
gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/UnifiedDiffTable.java
View File

@ -464,4 +464,16 @@ public class UnifiedDiffTable extends AbstractPatchContentTable {
m.addStyleName(Gerrit.RESOURCES.css().rightBorder());
m.closeTd();
}
@Override
void createFileCommentEditorOnSideA() {
}
@Override
void createFileCommentEditorOnSideB() {
}
@Override
void insertFileCommentRow(int row) {
}
}

4
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/SaveDraft.java
View File

@ -71,8 +71,8 @@ class SaveDraft extends Handler<PatchLineComment> {
final Account.Id me = currentUser.getAccountId();
if (comment.getKey().get() == null) {
if (comment.getLine() < 1) {
throw new IllegalStateException("Comment line must be >= 1, not "
if (comment.getLine() < 0) {
throw new IllegalStateException("Comment line must be >= 0, not "
+ comment.getLine());
}