From 44adfee873378af6d322b47121bdcfcc1423dc6f Mon Sep 17 00:00:00 2001 From: Michael Zhou Date: Tue, 26 Nov 2013 05:54:39 -0500 Subject: [PATCH] SideBySide2: Save all drafts when leaving / semi-auto focus on drafts Keep track of all unsaved drafts and save them when leaving the page. Using CodeMirror's "redraw" event, autofocus on the editArea of a Draftbox when scrolled into viewportMargin. This works smoothly for the common case where a user starts typing a draft, scrolls around to view the file, and scrolls back to continue typing. Even if the user forgets to scroll back and accidentally pressed "u" or "a", the draft will be saved, minimizing potential frustration. Change-Id: Idde88aaeb01e46f85734b48caad1de7c9b2ffba3 --- .../google/gerrit/client/diff/DraftBox.java | 16 +++-- .../gerrit/client/diff/SideBySide2.java | 58 +++++++++++++++++-- .../java/net/codemirror/lib/LineWidget.java | 6 ++ 3 files changed, 68 insertions(+), 12 deletions(-) diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.java index d5cab44392..1daee49e3f 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.java @@ -18,6 +18,7 @@ import com.google.gerrit.client.FormatUtil; import com.google.gerrit.client.changes.CommentApi; import com.google.gerrit.client.changes.CommentInfo; import com.google.gerrit.client.changes.CommentInput; +import com.google.gerrit.client.rpc.CallbackGroup; import com.google.gerrit.client.rpc.GerritCallback; import com.google.gerrit.client.ui.CommentLinkProcessor; import com.google.gerrit.reviewdb.client.PatchSet; @@ -175,7 +176,7 @@ class DraftBox extends CommentBox { resizePaddingWidget(); } - private boolean isEdit() { + boolean isEdit() { return UIObject.isVisible(p_edit); } @@ -205,6 +206,7 @@ class DraftBox extends CommentBox { } else { expandTimer.cancel(); } + parent.updateUnsaved(this, edit); resizePaddingWidget(); } @@ -257,10 +259,10 @@ class DraftBox extends CommentBox { @UiHandler("save") void onSave(ClickEvent e) { e.stopPropagation(); - onSave(); + save(null); } - private void onSave() { + void save(CallbackGroup group) { String message = editArea.getValue().trim(); if (message.length() == 0) { return; @@ -280,6 +282,7 @@ class DraftBox extends CommentBox { if (autoClosed) { setOpen(false); } + parent.updateUnsaved(DraftBox.this, false); } @Override @@ -289,9 +292,10 @@ class DraftBox extends CommentBox { } }; if (original.id() == null) { - CommentApi.createDraft(psId, input, cb); + CommentApi.createDraft(psId, input, group == null ? cb : group.add(cb)); } else { - CommentApi.updateDraft(psId, original.id(), input, cb); + CommentApi.updateDraft( + psId, original.id(), input, group == null ? cb : group.add(cb)); } getCm().focus(); } @@ -342,7 +346,7 @@ class DraftBox extends CommentBox { case 's': case 'S': e.preventDefault(); - onSave(); + save(null); return; } } else if (e.getNativeKeyCode() == KeyCodes.KEY_ESCAPE && !isDirty()) { diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java index c38a9c7ada..0a8f0e4841 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java @@ -87,8 +87,10 @@ import java.util.Collections; import java.util.Comparator; import java.util.EnumSet; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; public class SideBySide2 extends Screen { interface Binder extends UiBinder {} @@ -127,6 +129,7 @@ public class SideBySide2 extends Screen { private Map linePaddingOnOtherSideMap; private List diffChunks; private List skips; + private Set unsaved; private int context; private KeyCommandSet keysNavigation; @@ -150,6 +153,7 @@ public class SideBySide2 extends Screen { } context = pref.getContext(); + unsaved = new HashSet(); handlers = new ArrayList(6); // TODO: Re-implement necessary GlobalKey bindings. addDomHandler(GlobalKey.STOP_PROPAGATION, KeyPressEvent.getType()); @@ -248,6 +252,7 @@ public class SideBySide2 extends Screen { protected void onUnload() { super.onUnload(); + saveAllDrafts(null); removeKeyHandlerRegs(); if (resizeHandler != null) { resizeHandler.removeHandler(); @@ -602,7 +607,7 @@ public class SideBySide2 extends Screen { return box; } - CommentBox addCommentBox(CommentInfo info, CommentBox box) { + CommentBox addCommentBox(CommentInfo info, final CommentBox box) { box.setParent(this); diffTable.add(box); DisplaySide side = box.getSide(); @@ -649,6 +654,17 @@ public class SideBySide2 extends Screen { box instanceof DraftBox ? SidePanel.GutterType.DRAFT : SidePanel.GutterType.COMMENT)); + if (box instanceof DraftBox) { + boxWidget.onRedraw(new Runnable() { + @Override + public void run() { + DraftBox draftBox = (DraftBox) box; + if (draftBox.isEdit()) { + draftBox.editArea.setFocus(true); + } + } + }); + } return box; } @@ -659,6 +675,21 @@ public class SideBySide2 extends Screen { List list = linePublishedBoxesMap.get(handle); lineActiveBoxMap.put(handle, list.get(list.size() - 1)); } + unsaved.remove(box); + } + + void updateUnsaved(DraftBox box, boolean isUnsaved) { + if (isUnsaved) { + unsaved.add(box); + } else { + unsaved.remove(box); + } + } + + private void saveAllDrafts(CallbackGroup cb) { + for (DraftBox box : unsaved) { + box.save(cb); + } } void addFileCommentBox(CommentBox box) { @@ -1054,15 +1085,30 @@ public class SideBySide2 extends Screen { private Runnable upToChange(final boolean openReplyBox) { return new Runnable() { public void run() { - String b = base != null ? String.valueOf(base.get()) : null; - String rev = String.valueOf(revision.get()); - Gerrit.display( - PageLinks.toChange(changeId, rev), - new ChangeScreen2(changeId, b, rev, openReplyBox)); + if (unsaved.isEmpty()) { + goUpToChange(openReplyBox); + } else { + CallbackGroup group = new CallbackGroup(); + saveAllDrafts(group); + group.addFinal(new GerritCallback() { + @Override + public void onSuccess(Void result) { + goUpToChange(openReplyBox); + } + }).onSuccess(null); + } } }; } + private void goUpToChange(boolean openReplyBox) { + String b = base != null ? String.valueOf(base.get()) : null; + String rev = String.valueOf(revision.get()); + Gerrit.display( + PageLinks.toChange(changeId, rev), + new ChangeScreen2(changeId, b, rev, openReplyBox)); + } + private Runnable openClosePublished(final CodeMirror cm) { return new Runnable() { @Override diff --git a/gerrit-gwtui/src/main/java/net/codemirror/lib/LineWidget.java b/gerrit-gwtui/src/main/java/net/codemirror/lib/LineWidget.java index bd1e0a3445..b6ea1aab17 100644 --- a/gerrit-gwtui/src/main/java/net/codemirror/lib/LineWidget.java +++ b/gerrit-gwtui/src/main/java/net/codemirror/lib/LineWidget.java @@ -25,6 +25,12 @@ public class LineWidget extends JavaScriptObject { public final native void clear() /*-{ this.clear(); }-*/; public final native void changed() /*-{ this.changed(); }-*/; + public final native void onRedraw(Runnable thunk) /*-{ + this.on("redraw", $entry(function() { + thunk.@java.lang.Runnable::run()(); + })); + }-*/; + public final native void onFirstRedraw(Runnable thunk) /*-{ var w = this; var h = $entry(function() {