opendev/gerrit
Code Issues Proposed changes

DiffScreen: Respect the "Skip Uncommented" diff preference

Browse Source 
The effect of this preference wasn't migrated from the old diff screen
to the current one, and the prefernce hasn't been available in the diff
preferences box, but the preference itself was migrated from ReviewDb to
NoteDb (the Git backend).

This change updates the diff screen header to respect this preference.
Now that the side-by-side and the unified diff screens share the same
code base, this feature is made available in both of them.

Note that currently, the navigation links and keyboard shortcuts are not
updated upon toggling the option in PreferencesBox: they are only
updated when the preference is saved and the diff screen is reloaded.

To support updating without refreshing, we will need to replace the
KeyCommand in GlobalKey, which is of low priority and will be
implemented in a future change.

Bug: Issue 3446
Change-Id: Ia594703c9fe7580387d6efb5f5e1011e1ceb5f7d
This commit is contained in:
Michael Zhou
2016-04-11 06:47:19 -04:00
parent 9ee5cf15dc
commit d94cf520b0
7 changed files with 57 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentsCollections.java
View File

@@ -29,17 +29,23 @@ import java.util.Comparator;
/** Collection of published and draft comments loaded from the server. */
class CommentsCollections {
private String path;
private final String path;
private final PatchSet.Id base;
private final PatchSet.Id revision;
private NativeMap<JsArray<CommentInfo>> publishedBaseAll;
private NativeMap<JsArray<CommentInfo>> publishedRevisionAll;
JsArray<CommentInfo> publishedBase;
JsArray<CommentInfo> publishedRevision;
JsArray<CommentInfo> draftsBase;
JsArray<CommentInfo> draftsRevision;
void load(PatchSet.Id base, PatchSet.Id revision, String path,
CallbackGroup group) {
CommentsCollections(PatchSet.Id base, PatchSet.Id revision, String path) {
this.path = path;
this.base = base;
this.revision = revision;
}
void load(CallbackGroup group) {
if (base != null) {
CommentApi.comments(base, group.add(publishedBase()));
}
@@ -53,10 +59,25 @@ class CommentsCollections {
}
}
boolean hasCommentForPath(String filePath) {
if (base != null) {
JsArray<CommentInfo> forBase = publishedBaseAll.get(filePath);
if (forBase != null && forBase.length() > 0) {
return true;
}
}
JsArray<CommentInfo> forRevision = publishedRevisionAll.get(filePath);
if (forRevision != null && forRevision.length() > 0) {
return true;
}
return false;
}
private AsyncCallback<NativeMap<JsArray<CommentInfo>>> publishedBase() {
return new AsyncCallback<NativeMap<JsArray<CommentInfo>>>() {
@Override
public void onSuccess(NativeMap<JsArray<CommentInfo>> result) {
publishedBaseAll = result;
publishedBase = sort(result.get(path));
}
@@ -70,6 +91,7 @@ class CommentsCollections {
return new AsyncCallback<NativeMap<JsArray<CommentInfo>>>() {
@Override
public void onSuccess(NativeMap<JsArray<CommentInfo>> result) {
publishedRevisionAll = result;
publishedRevision = sort(result.get(path));
}

5
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffScreen.java
View File

@@ -209,8 +209,9 @@ abstract class DiffScreen extends Screen {
}));
}
final CommentsCollections comments = new CommentsCollections();
comments.load(base, revision, path, group2);
final CommentsCollections comments =
new CommentsCollections(base, revision, path);
comments.load(group2);
RestApi call = ChangeApi.detail(changeId.get());
ChangeList.addOptions(call, EnumSet.of(

16
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java
View File

@@ -97,6 +97,7 @@ public class Header extends Composite {
private boolean hasPrev;
private boolean hasNext;
private String nextPath;
private JsArray<FileInfo> files;
private PreferencesAction prefsAction;
private ReviewedState reviewedState;
@@ -161,12 +162,11 @@ public class Header extends Composite {
DiffApi.list(patchSetId, base, new GerritCallback<NativeMap<FileInfo>>() {
@Override
public void onSuccess(NativeMap<FileInfo> result) {
JsArray<FileInfo> files = result.values();
files = result.values();
FileInfo.sortFileInfoByPath(files);
fileNumber.setInnerText(
Integer.toString(Natives.asList(files).indexOf(result.get(path)) + 1));
fileCount.setInnerText(Integer.toString(files.length()));
setupPrevNextFiles(files, findCurrentFileIndex(files));
}
});
@@ -300,12 +300,18 @@ public class Header extends Composite {
}
}
void setupPrevNextFiles(JsArray<FileInfo> files, int currIndex) {
private boolean shouldSkipFile(FileInfo curr, CommentsCollections comments) {
return prefs.skipDeleted() && ChangeType.DELETED.matches(curr.status())
|| prefs.skipUncommented() && !comments.hasCommentForPath(curr.path());
}
void setupPrevNextFiles(CommentsCollections comments) {
FileInfo prevInfo = null;
FileInfo nextInfo = null;
int currIndex = findCurrentFileIndex(files);
for (int i = currIndex - 1; i >= 0; i--) {
FileInfo curr = files.get(i);
if (prefs.skipDeleted() && ChangeType.DELETED.matches(curr.status())) {
if (shouldSkipFile(curr, comments)) {
continue;
} else {
prevInfo = curr;
@@ -314,7 +320,7 @@ public class Header extends Composite {
}
for (int i = currIndex + 1; i < files.length(); i++) {
FileInfo curr = files.get(i);
if (prefs.skipDeleted() && ChangeType.DELETED.matches(curr.status())) {
if (shouldSkipFile(curr, comments)) {
continue;
} else {
nextInfo = curr;

8
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PreferencesBox.java
View File

@@ -103,6 +103,7 @@ public class PreferencesBox extends Composite {
@UiField ToggleButton renderEntireFile;
@UiField ToggleButton matchBrackets;
@UiField ToggleButton skipDeleted;
@UiField ToggleButton skipUncommented;
@UiField ListBox theme;
@UiField Element modeLabel;
@UiField ListBox mode;
@@ -196,6 +197,7 @@ public class PreferencesBox extends Composite {
expandAllComments.setValue(prefs.expandAllComments());
matchBrackets.setValue(prefs.matchBrackets());
skipDeleted.setValue(!prefs.skipDeleted());
skipUncommented.setValue(!prefs.skipUncommented());
setTheme(prefs.theme());
if (view == null || view.canRenderEntireFile(prefs)) {
@@ -505,6 +507,12 @@ public class PreferencesBox extends Composite {
// TODO: Update the navigation links on the current DiffScreen
}
@UiHandler("skipUncommented")
void onSkipUncommented(ValueChangeEvent<Boolean> e) {
prefs.skipUncommented(!e.getValue());
// TODO: Update the navigation links on the current DiffScreen
}
@UiHandler("theme")
void onTheme(@SuppressWarnings("unused") ChangeEvent e) {
final Theme newTheme = getSelectedTheme();

7
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PreferencesBox.ui.xml
View File

@@ -303,6 +303,13 @@ limitations under the License.
<g:downFace><ui:msg>No</ui:msg></g:downFace>
</g:ToggleButton></td>
</tr>
<tr>
<th><ui:msg>Skip Uncommented Files</ui:msg></th>
<td><g:ToggleButton ui:field='skipUncommented'>
<g:upFace><ui:msg>Yes</ui:msg></g:upFace>
<g:downFace><ui:msg>No</ui:msg></g:downFace>
</g:ToggleButton></td>
</tr>
<tr>
<td></td>
<td>

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide.java
View File

@@ -94,6 +94,7 @@ public class SideBySide extends DiffScreen {
getChangeStatus().isOpen());
setTheme(result.getTheme());
display(comments);
header.setupPrevNextFiles(comments);
}
};
}

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Unified.java
View File

@@ -98,6 +98,7 @@ public class Unified extends DiffScreen {
getChangeStatus().isOpen());
setTheme(result.getTheme());
display(comments);
header.setupPrevNextFiles(comments);
}
};
}