opendev/gerrit
Code Issues Proposed changes

Refactor CommentedChangeActionDialog to be more reusable

Browse Source 
This dialog was refactored out originally to be used
by the revert command, then it got used by the restore
command.  The dialog became a bit messy with way too
many arguments to the constructor and some fields which
were only used once.

This change reduces some of the dialog's functionality
in favor of allowing differentiation to be done via
extending the class instead.  This change also chooses
a few more common defaults for the dialog making it a
bit less cumbersome to setup.  And finally, since the
dialog is a bit more generic and no longer tied to a
change, the file is moved from the change package to the
ui package.

Change-Id: Ic6deabadf4aa983c6632517168fead93623e66f7
This commit is contained in:
Martin Fick
2012-01-09 14:17:56 -07:00
parent 772839afa4
commit b61aff0702
9 changed files with 124 additions and 148 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -18,10 +18,6 @@ import com.google.gwt.resources.client.CssResource;
public interface GerritCss extends CssResource {
String greenCheckClass();
String abandonChangeDialog();
String abandonMessage();
String revertChangeDialog();
String revertMessage();
String accountContactOnFile();
String accountContactPrivacyDetails();
String accountDashboard();
@@ -58,6 +54,8 @@ public interface GerritCss extends CssResource {
String changeTypeCell();
String changeid();
String closedstate();
String commentedActionDialog();
String commentedActionMessage();
String commentCell();
String commentEditorPanel();
String commentHolder();

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

@@ -113,13 +113,11 @@ public interface ChangeConstants extends Constants {
String buttonRevertChangeBegin();
String buttonRevertChangeSend();
String buttonRevertChangeCancel();
String headingRevertMessage();
String revertChangeTitle();
String buttonAbandonChangeBegin();
String buttonAbandonChangeSend();
String buttonAbandonChangeCancel();
String headingAbandonMessage();
String abandonChangeTitle();
String oldVersionHistory();
@@ -135,7 +133,6 @@ public interface ChangeConstants extends Constants {
String buttonRestoreChangeBegin();
String restoreChangeTitle();
String buttonRestoreChangeCancel();
String headingRestoreMessage();
String buttonRestoreChangeSend();

3
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties
View File

@@ -90,7 +90,6 @@ initialCommit = Initial Commit
buttonAbandonChangeBegin = Abandon Change
buttonAbandonChangeSend = Abandon Change
buttonAbandonChangeCancel = Cancel
headingAbandonMessage = Abandon Message:
abandonChangeTitle = Code Review - Abandon Change
oldVersionHistory = Old Version History:
@@ -99,13 +98,11 @@ autoMerge = Auto Merge
buttonRevertChangeBegin = Revert Change
buttonRevertChangeSend = Revert Change
buttonRevertChangeCancel = Cancel
headingRevertMessage = Revert Commit Message:
revertChangeTitle = Code Review - Revert Merged Change
buttonRestoreChangeBegin = Restore Change
restoreChangeTitle = Code Review - Restore Change
buttonRestoreChangeCancel = Cancel
headingRestoreMessage = Restore Message:
buttonRestoreChangeSend = Restore Change

101
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java
View File

@@ -20,6 +20,7 @@ import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.patches.PatchUtil;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.AccountDashboardLink;
import com.google.gerrit.client.ui.CommentedActionDialog;
import com.google.gerrit.client.ui.ComplexDisclosurePanel;
import com.google.gerrit.client.ui.ListenableAccountDiffPreference;
import com.google.gerrit.common.PageLinks;
@@ -49,6 +50,7 @@ import com.google.gwt.user.client.ui.Anchor;
import com.google.gwt.user.client.ui.Button;
import com.google.gwt.user.client.ui.DisclosurePanel;
import com.google.gwt.user.client.ui.FlowPanel;
import com.google.gwt.user.client.ui.FocusWidget;
import com.google.gwt.user.client.ui.Grid;
import com.google.gwt.user.client.ui.HTMLTable.CellFormatter;
import com.google.gwt.user.client.ui.InlineLabel;
@@ -60,7 +62,8 @@ import java.util.HashSet;
import java.util.List;
import java.util.Set;
class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel implements OpenHandler<DisclosurePanel> {
class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel
implements OpenHandler<DisclosurePanel> {
private static final int R_AUTHOR = 0;
private static final int R_COMMITTER = 1;
private static final int R_PARENTS = 2;
@@ -467,15 +470,22 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel implements O
@Override
public void onClick(final ClickEvent event) {
b.setEnabled(false);
new CommentedChangeActionDialog(patchSet.getId(), createCommentedCallback(b, true),
Util.C.revertChangeTitle(), Util.C.headingRevertMessage(),
Util.C.buttonRevertChangeSend(), Util.C.buttonRevertChangeCancel(),
Gerrit.RESOURCES.css().revertChangeDialog(), Gerrit.RESOURCES.css().revertMessage(),
Util.M.revertChangeDefaultMessage(detail.getInfo().getSubject(), detail.getPatchSet().getRevision().get())) {
public void onSend() {
Util.MANAGE_SVC.revertChange(getPatchSetId() , getMessageText(), createCallback());
}
}.center();
new ActionDialog(b, true, Util.C.revertChangeTitle(),
Util.C.headingRevertMessage()) {
{
sendButton.setText(Util.C.buttonRevertChangeSend());
message.setText(Util.M.revertChangeDefaultMessage(
detail.getInfo().getSubject(),
detail.getPatchSet().getRevision().get())
);
}
@Override
public void onSend() {
Util.MANAGE_SVC.revertChange(patchSet.getId(), getMessageText(),
createCallback());
}
}.center();
}
});
actionsPanel.add(b);
@@ -487,14 +497,18 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel implements O
@Override
public void onClick(final ClickEvent event) {
b.setEnabled(false);
new CommentedChangeActionDialog(patchSet.getId(), createCommentedCallback(b, false),
Util.C.abandonChangeTitle(), Util.C.headingAbandonMessage(),
Util.C.buttonAbandonChangeSend(), Util.C.buttonAbandonChangeCancel(),
Gerrit.RESOURCES.css().abandonChangeDialog(), Gerrit.RESOURCES.css().abandonMessage()) {
public void onSend() {
Util.MANAGE_SVC.abandonChange(getPatchSetId() , getMessageText(), createCallback());
}
}.center();
new ActionDialog(b, false, Util.C.abandonChangeTitle(),
Util.C.headingAbandonMessage()) {
{
sendButton.setText(Util.C.buttonAbandonChangeSend());
}
@Override
public void onSend() {
Util.MANAGE_SVC.abandonChange(patchSet.getId(), getMessageText(),
createCallback());
}
}.center();
}
});
actionsPanel.add(b);
@@ -530,14 +544,18 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel implements O
@Override
public void onClick(final ClickEvent event) {
b.setEnabled(false);
new CommentedChangeActionDialog(patchSet.getId(), createCommentedCallback(b, false),
Util.C.restoreChangeTitle(), Util.C.headingRestoreMessage(),
Util.C.buttonRestoreChangeSend(), Util.C.buttonRestoreChangeCancel(),
Gerrit.RESOURCES.css().abandonChangeDialog(), Gerrit.RESOURCES.css().abandonMessage()) {
public void onSend() {
Util.MANAGE_SVC.restoreChange(getPatchSetId(), getMessageText(), createCallback());
}
}.center();
new ActionDialog(b, false, Util.C.restoreChangeTitle(),
Util.C.headingRestoreMessage()) {
{
sendButton.setText(Util.C.buttonRestoreChangeSend());
}
@Override
public void onSend() {
Util.MANAGE_SVC.restoreChange(patchSet.getId(), getMessageText(),
createCallback());
}
}.center();
}
});
actionsPanel.add(b);
@@ -736,19 +754,24 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel implements O
}
}
private AsyncCallback<ChangeDetail> createCommentedCallback(final Button b, final boolean redirect) {
return new AsyncCallback<ChangeDetail>() {
public void onSuccess(ChangeDetail result) {
if (redirect) {
Gerrit.display(PageLinks.toChange(result.getChange().getId()));
} else {
changeScreen.update(result);
}
}
private abstract class ActionDialog extends CommentedActionDialog<ChangeDetail> {
public ActionDialog(final FocusWidget enableOnFailure, final boolean redirect,
String dialogTitle, String dialogHeading) {
super(dialogTitle, dialogHeading, new AsyncCallback<ChangeDetail>() {
@Override
public void onSuccess(ChangeDetail result) {
if (redirect) {
Gerrit.display(PageLinks.toChange(result.getChange().getId()));
} else {
changeScreen.update(result);
}
}
public void onFailure(Throwable caught) {
b.setEnabled(true);
}
};
@Override
public void onFailure(Throwable caught) {
enableOnFailure.setEnabled(true);
}
});
}
}
}

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

@@ -1209,7 +1209,6 @@ a:hover.downloadLink {
/** PublishCommentsScreen **/
.publishCommentsScreen .smallHeading {
font-size: small;
font-weight: bold;
@@ -1259,57 +1258,29 @@ a:hover.downloadLink {
}
/** AbandonChangeDialog **/
.abandonChangeDialog .gwt-DisclosurePanel .header td {
/** CommentedActionDialog **/
.commentedActionDialog .gwt-DisclosurePanel .header td {
font-weight: bold;
white-space: nowrap;
}
.abandonChangeDialog .smallHeading {
.commentedActionDialog .smallHeading {
font-size: small;
font-weight: bold;
white-space: nowrap;
}
.abandonChangeDialog .abandonMessage {
.commentedActionDialog .commentedActionMessage {
margin-left: 10px;
background: trimColor;
padding: 5px 5px 5px 5px;
}
.abandonChangeDialog .abandonMessage textarea {
.commentedActionDialog .commentedActionMessage textarea {
font-size: small;
}
.abandonChangeDialog .gwt-Hyperlink {
.commentedActionDialog .gwt-Hyperlink {
white-space: nowrap;
font-size: small;
}
/** RevertChangeDialog **/
.revertChangeDialog .gwt-DisclosurePanel .header td {
font-weight: bold;
white-space: nowrap;
}
.revertChangeDialog .smallHeading {
font-size: small;
font-weight: bold;
white-space: nowrap;
}
.revertChangeDialog .revertMessage {
margin-left: 10px;
background: trimColor;
padding: 5px 5px 5px 5px;
}
.revertChangeDialog .revertMessage textarea {
font-size: small;
}
.revertChangeDialog .gwt-Hyperlink {
white-space: nowrap;
font-size: small;
}
/** PatchBrowserPopup **/
.patchBrowserPopup {
opacity: 0.90;

108
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentedChangeActionDialog.java → gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/CommentedActionDialog.java
View File

@@ -12,12 +12,11 @@
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.client.changes;
package com.google.gerrit.client.ui;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.SmallHeading;
import com.google.gerrit.common.data.ChangeDetail;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gwt.event.dom.client.ClickEvent;
import com.google.gwt.event.dom.client.ClickHandler;
import com.google.gwt.event.logical.shared.CloseEvent;
@@ -31,83 +30,72 @@ import com.google.gwtexpui.globalkey.client.GlobalKey;
import com.google.gwtexpui.globalkey.client.NpTextArea;
import com.google.gwtexpui.user.client.AutoCenterDialogBox;
public abstract class CommentedChangeActionDialog extends AutoCenterDialogBox implements CloseHandler<PopupPanel>{
private final FlowPanel panel;
private final NpTextArea message;
private final Button sendButton;
private final Button cancelButton;
private final PatchSet.Id psid;
private final AsyncCallback<ChangeDetail> callback;
public abstract class CommentedActionDialog<T> extends AutoCenterDialogBox
implements CloseHandler<PopupPanel> {
protected final FlowPanel panel;
protected final NpTextArea message;
protected final Button sendButton;
protected final Button cancelButton;
protected final FlowPanel buttonPanel;
protected AsyncCallback<T> callback;
private boolean buttonClicked = false;
protected boolean sent = false;
public CommentedChangeActionDialog(final PatchSet.Id psi,
final AsyncCallback<ChangeDetail> callback, final String dialogTitle,
final String dialogHeading, final String buttonSend,
final String buttonCancel, final String dialogStyle,
final String messageStyle) {
this(psi, callback, dialogTitle, dialogHeading, buttonSend, buttonCancel, dialogStyle, messageStyle, null);
}
public CommentedChangeActionDialog(final PatchSet.Id psi,
final AsyncCallback<ChangeDetail> callback, final String dialogTitle,
final String dialogHeading, final String buttonSend,
final String buttonCancel, final String dialogStyle,
final String messageStyle, final String defaultMessage) {
public CommentedActionDialog(final String title, final String heading,
AsyncCallback<T> callback) {
super(/* auto hide */false, /* modal */true);
setGlassEnabled(true);
psid = psi;
this.callback = callback;
addStyleName(dialogStyle);
setText(dialogTitle);
panel = new FlowPanel();
add(panel);
setGlassEnabled(true);
setText(title);
panel.add(new SmallHeading(dialogHeading));
final FlowPanel mwrap = new FlowPanel();
mwrap.setStyleName(messageStyle);
panel.add(mwrap);
addStyleName(Gerrit.RESOURCES.css().commentedActionDialog());
message = new NpTextArea();
message.setCharacterWidth(60);
message.setVisibleLines(10);
message.setText(defaultMessage);
DOM.setElementPropertyBoolean(message.getElement(), "spellcheck", true);
mwrap.add(message);
final FlowPanel buttonPanel = new FlowPanel();
panel.add(buttonPanel);
sendButton = new Button(buttonSend);
sendButton = new Button(Util.C.commentedActionButtonSend());
sendButton.addClickHandler(new ClickHandler() {
@Override
public void onClick(final ClickEvent event) {
sendButton.setEnabled(false);
cancelButton.setEnabled(false);
enableButtons(false);
onSend();
}
});
buttonPanel.add(sendButton);
cancelButton = new Button(buttonCancel);
cancelButton = new Button(Util.C.commentedActionButtonCancel());
DOM.setStyleAttribute(cancelButton.getElement(), "marginLeft", "300px");
cancelButton.addClickHandler(new ClickHandler() {
@Override
public void onClick(final ClickEvent event) {
buttonClicked = true;
if (callback != null) {
callback.onFailure(null);
}
hide();
}
});
final FlowPanel mwrap = new FlowPanel();
mwrap.setStyleName(Gerrit.RESOURCES.css().commentedActionMessage());
mwrap.add(message);
buttonPanel = new FlowPanel();
buttonPanel.add(sendButton);
buttonPanel.add(cancelButton);
panel = new FlowPanel();
panel.add(new SmallHeading(heading));
panel.add(mwrap);
panel.add(buttonPanel);
add(panel);
addCloseHandler(this);
}
public void enableButtons(boolean enable) {
sendButton.setEnabled(enable);
cancelButton.setEnabled(enable);
}
@Override
public void center() {
super.center();
@@ -117,30 +105,27 @@ public abstract class CommentedChangeActionDialog extends AutoCenterDialogBox im
@Override
public void onClose(CloseEvent<PopupPanel> event) {
if (!buttonClicked) {
// the dialog was closed without one of the buttons being pressed
// e.g. the user pressed ESC to close the dialog
if (!sent) {
// the dialog was closed without the send button being pressed
// e.g. the user pressed Cancel or ESC to close the dialog
if (callback != null) {
callback.onFailure(null);
}
}
sent = false;
}
public abstract void onSend();
public PatchSet.Id getPatchSetId() {
return psid;
}
public String getMessageText() {
return message.getText().trim();
}
public GerritCallback<ChangeDetail> createCallback() {
return new GerritCallback<ChangeDetail>(){
public AsyncCallback<T> createCallback() {
return new GerritCallback<T>(){
@Override
public void onSuccess(ChangeDetail result) {
buttonClicked = true;
public void onSuccess(T result) {
sent = true;
if (callback != null) {
callback.onSuccess(result);
}
@@ -149,8 +134,7 @@ public abstract class CommentedChangeActionDialog extends AutoCenterDialogBox im
@Override
public void onFailure(Throwable caught) {
sendButton.setEnabled(true);
cancelButton.setEnabled(true);
enableButtons(true);
super.onFailure(caught);
}
};

5
gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/ProjectConstants.java → gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/UIConstants.java
View File

@@ -16,7 +16,10 @@ package com.google.gerrit.client.ui;
import com.google.gwt.i18n.client.Constants;
public interface ProjectConstants extends Constants {
public interface UIConstants extends Constants {
String commentedActionButtonSend();
String commentedActionButtonCancel();
String projectName();
String projectDescription();
String projectListOpen();

3
gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/ProjectConstants.properties → gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/UIConstants.properties
View File

@@ -1,3 +1,6 @@
commentedActionButtonSend = Submit
commentedActionButtonCancel = Cancel
projectName = Project Name
projectDescription = Project Description
projectListOpen = Select project

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

@@ -17,5 +17,5 @@ package com.google.gerrit.client.ui;
import com.google.gwt.core.client.GWT;
public class Util {
public static final ProjectConstants C = GWT.create(ProjectConstants.class);
public static final UIConstants C = GWT.create(UIConstants.class);
}