opendev/gerrit
Code Issues Proposed changes

Replace TextSaveButtonListener use with new OnEditEnabler.

Browse Source 
The TextSaveButtonListener class was used to enable a button
by listening to keystrokes to determine whether actual text
had been entered in the field or not.  This is used to enable
save buttons in forms.  The method used to detect changes was
very efficient, but it failed to catch CTRL-V pasting in
Chrome and X11 middle button pasting in FF and Chrome.

Replace the uses of TextSaveButtonListener with a new
OnEditEnabler class.  This class is designed to listen to
several input widgets types, ListBoxes and CheckBoxes, not
just textboxes, making them also able to enable the
FocusWidget.  This class can easily be extended to handle
other types of input widgets if neccessary.  Instead of just
listening to keystrokes from textboxes, OnEditEnabler
registers a listener for key press, key down, and mouse up.
On receipt of those events, it compares the new text against
the old text (the text before getting focus) to determine if
any of the events have actually changed the  text. This seems
able to capture text changes more reliably.

Change-Id: I285cea25e3ef3d076def06f240bf678857595ae3
This commit is contained in:
Martin Fick
2010-09-21 08:23:07 -06:00
committed by Shawn O. Pearce
parent e3febd9e54
commit 6f5d33e7c1
8 changed files with 196 additions and 121 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
gerrit-gwtui/src/main/java/com/google/gerrit/client/account/ContactPanelFull.java
View File

@@ -15,7 +15,7 @@
package com.google.gerrit.client.account;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.ui.TextSaveButtonListener;
import com.google.gerrit.client.ui.OnEditEnabler;
import com.google.gerrit.reviewdb.Account;
import com.google.gerrit.reviewdb.ContactInformation;
import com.google.gwt.user.client.ui.Grid;
@@ -80,11 +80,11 @@ class ContactPanelFull extends ContactPanelShort {
infoSecure.getCellFormatter().addStyleName(0, 1, Gerrit.RESOURCES.css().topmost());
infoSecure.getCellFormatter().addStyleName(3, 0, Gerrit.RESOURCES.css().bottomheader());
final TextSaveButtonListener sbl = new TextSaveButtonListener(save);
addressTxt.addKeyPressHandler(sbl);
countryTxt.addKeyPressHandler(sbl);
phoneTxt.addKeyPressHandler(sbl);
faxTxt.addKeyPressHandler(sbl);
final OnEditEnabler sbl = new OnEditEnabler(save);
sbl.listenTo(addressTxt);
sbl.listenTo(countryTxt);
sbl.listenTo(phoneTxt);
sbl.listenTo(faxTxt);
}
@Override

5
gerrit-gwtui/src/main/java/com/google/gerrit/client/account/ContactPanelShort.java
View File

@@ -16,7 +16,7 @@ package com.google.gerrit.client.account;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.TextSaveButtonListener;
import com.google.gerrit.client.ui.OnEditEnabler;
import com.google.gerrit.reviewdb.Account;
import com.google.gerrit.reviewdb.AccountExternalId;
import com.google.gerrit.reviewdb.ContactInformation;
@@ -120,9 +120,8 @@ class ContactPanelShort extends Composite {
doSave();
}
});
new OnEditEnabler(save, nameTxt);
final TextSaveButtonListener sbl = new TextSaveButtonListener(save);
nameTxt.addKeyPressHandler(sbl);
emailPick.addChangeHandler(new ChangeHandler() {
@Override
public void onChange(final ChangeEvent event) {

4
gerrit-gwtui/src/main/java/com/google/gerrit/client/account/NewAgreementScreen.java
View File

@@ -18,8 +18,8 @@ import com.google.gerrit.client.ErrorDialog;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.AccountScreen;
import com.google.gerrit.client.ui.OnEditEnabler;
import com.google.gerrit.client.ui.SmallHeading;
import com.google.gerrit.client.ui.TextSaveButtonListener;
import com.google.gerrit.common.PageLinks;
import com.google.gerrit.common.data.AgreementInfo;
import com.google.gerrit.reviewdb.AccountAgreement;
@@ -144,7 +144,7 @@ public class NewAgreementScreen extends AccountScreen {
});
finalGroup.add(submit);
formBody.add(finalGroup);
new TextSaveButtonListener(yesIAgreeBox, submit);
new OnEditEnabler(submit, yesIAgreeBox);
final FormPanel form = new FormPanel();
form.add(formBody);

4
gerrit-gwtui/src/main/java/com/google/gerrit/client/account/UsernameField.java
View File

@@ -17,7 +17,7 @@ package com.google.gerrit.client.account;
import com.google.gerrit.client.ErrorDialog;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.TextSaveButtonListener;
import com.google.gerrit.client.ui.OnEditEnabler;
import com.google.gerrit.common.errors.InvalidUserNameException;
import com.google.gerrit.reviewdb.Account;
import com.google.gwt.event.dom.client.ClickEvent;
@@ -73,7 +73,7 @@ class UsernameField extends Composite {
doSetUserName();
}
});
new TextSaveButtonListener(userNameTxt, setUserName);
new OnEditEnabler(setUserName, userNameTxt);
userNameLbl.setVisible(false);
body.add(userNameLbl);

8
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java
View File

@@ -22,9 +22,9 @@ import com.google.gerrit.client.ui.AccountGroupSuggestOracle;
import com.google.gerrit.client.ui.AccountScreen;
import com.google.gerrit.client.ui.AddMemberBox;
import com.google.gerrit.client.ui.FancyFlexTable;
import com.google.gerrit.client.ui.OnEditEnabler;
import com.google.gerrit.client.ui.RPCSuggestOracle;
import com.google.gerrit.client.ui.SmallHeading;
import com.google.gerrit.client.ui.TextSaveButtonListener;
import com.google.gerrit.common.data.AccountInfoCache;
import com.google.gerrit.common.data.GroupDetail;
import com.google.gerrit.reviewdb.Account;
@@ -135,7 +135,7 @@ public class AccountGroupScreen extends AccountScreen {
groupNamePanel.add(saveName);
add(groupNamePanel);
new TextSaveButtonListener(groupNameTxt, saveName);
new OnEditEnabler(saveName, groupNameTxt);
}
private void initOwner() {
@@ -167,7 +167,7 @@ public class AccountGroupScreen extends AccountScreen {
ownerPanel.add(saveOwner);
add(ownerPanel);
new TextSaveButtonListener(ownerTxtBox, saveOwner);
new OnEditEnabler(saveOwner, ownerTxtBox);
}
private void initDescription() {
@@ -196,7 +196,7 @@ public class AccountGroupScreen extends AccountScreen {
vp.add(saveDesc);
add(vp);
new TextSaveButtonListener(descTxt, saveDesc);
new OnEditEnabler(saveDesc, descTxt);
}
private void initGroupType() {

44
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectInfoScreen.java
View File

@@ -17,16 +17,12 @@ package com.google.gerrit.client.admin;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.rpc.ScreenLoadCallback;
import com.google.gerrit.client.ui.OnEditEnabler;
import com.google.gerrit.client.ui.SmallHeading;
import com.google.gerrit.client.ui.TextSaveButtonListener;
import com.google.gerrit.common.data.ProjectDetail;
import com.google.gerrit.reviewdb.Project;
import com.google.gwt.event.dom.client.ChangeEvent;
import com.google.gwt.event.dom.client.ChangeHandler;
import com.google.gwt.event.dom.client.ClickEvent;
import com.google.gwt.event.dom.client.ClickHandler;
import com.google.gwt.event.logical.shared.ValueChangeEvent;
import com.google.gwt.event.logical.shared.ValueChangeHandler;
import com.google.gwt.user.client.ui.Button;
import com.google.gwt.user.client.ui.CheckBox;
import com.google.gwt.user.client.ui.ListBox;
@@ -48,6 +44,8 @@ public class ProjectInfoScreen extends ProjectScreen {
private NpTextArea descTxt;
private Button saveProject;
private OnEditEnabler saveEnabler;
public ProjectInfoScreen(final Project.NameKey toShow) {
super(toShow);
}
@@ -82,7 +80,6 @@ public class ProjectInfoScreen extends ProjectScreen {
result.canModifyAgreements ||
result.canModifyDescription ||
result.canModifyMergeType);
saveProject.setEnabled(false);
display(result);
}
});
@@ -109,18 +106,11 @@ public class ProjectInfoScreen extends ProjectScreen {
vp.add(descTxt);
add(vp);
new TextSaveButtonListener(descTxt, saveProject);
saveEnabler = new OnEditEnabler(saveProject);
saveEnabler.listenTo(descTxt);
}
private void initProjectOptions() {
final ValueChangeHandler<Boolean> onChangeSave =
new ValueChangeHandler<Boolean>() {
@Override
public void onValueChange(ValueChangeEvent<Boolean> event) {
saveProject.setEnabled(true);
}
};
projectOptionsPanel = new VerticalPanel();
projectOptionsPanel.add(new SmallHeading(Util.C.headingProjectOptions()));
@@ -128,39 +118,26 @@ public class ProjectInfoScreen extends ProjectScreen {
for (final Project.SubmitType type : Project.SubmitType.values()) {
submitType.addItem(Util.toLongString(type), type.name());
}
submitType.addChangeHandler(new ChangeHandler() {
@Override
public void onChange(final ChangeEvent event) {
saveProject.setEnabled(true);
}
});
saveEnabler.listenTo(submitType);
projectOptionsPanel.add(submitType);
requireChangeID = new CheckBox(Util.C.requireChangeID(), true);
requireChangeID.addValueChangeHandler(onChangeSave);
saveEnabler.listenTo(requireChangeID);
projectOptionsPanel.add(requireChangeID);
add(projectOptionsPanel);
}
private void initAgreements() {
final ValueChangeHandler<Boolean> onChangeSave =
new ValueChangeHandler<Boolean>() {
@Override
public void onValueChange(ValueChangeEvent<Boolean> event) {
saveProject.setEnabled(true);
}
};
agreementsPanel = new VerticalPanel();
agreementsPanel.add(new SmallHeading(Util.C.headingAgreements()));
useContributorAgreements = new CheckBox(Util.C.useContributorAgreements());
useContributorAgreements.addValueChangeHandler(onChangeSave);
saveEnabler.listenTo(useContributorAgreements);
agreementsPanel.add(useContributorAgreements);
useSignedOffBy = new CheckBox(Util.C.useSignedOffBy(), true);
useSignedOffBy.addValueChangeHandler(onChangeSave);
saveEnabler.listenTo(useSignedOffBy);
agreementsPanel.add(useSignedOffBy);
add(agreementsPanel);
@@ -193,6 +170,8 @@ public class ProjectInfoScreen extends ProjectScreen {
useSignedOffBy.setValue(project.isUseSignedOffBy());
requireChangeID.setValue(project.isRequireChangeID());
setSubmitType(project.getSubmitType());
saveProject.setEnabled(false);
}
private void doSave() {
@@ -206,7 +185,6 @@ public class ProjectInfoScreen extends ProjectScreen {
}
enableForm(false, false, false);
saveProject.setEnabled(false);
Util.PROJECT_SVC.changeProjectSettings(project,
new GerritCallback<ProjectDetail>() {

169
gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/OnEditEnabler.java Normal file
View File

@@ -0,0 +1,169 @@
// Copyright (C) 2010 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.client.ui;
import com.google.gwt.event.dom.client.ChangeEvent;
import com.google.gwt.event.dom.client.ChangeHandler;
import com.google.gwt.event.dom.client.FocusEvent;
import com.google.gwt.event.dom.client.FocusHandler;
import com.google.gwt.event.dom.client.KeyDownEvent;
import com.google.gwt.event.dom.client.KeyDownHandler;
import com.google.gwt.event.dom.client.KeyPressEvent;
import com.google.gwt.event.dom.client.KeyPressHandler;
import com.google.gwt.event.dom.client.MouseUpEvent;
import com.google.gwt.event.dom.client.MouseUpHandler;
import com.google.gwt.event.logical.shared.ValueChangeEvent;
import com.google.gwt.event.logical.shared.ValueChangeHandler;
import com.google.gwt.event.shared.GwtEvent;
import com.google.gwt.user.client.Command;
import com.google.gwt.user.client.DeferredCommand;
import com.google.gwt.user.client.ui.CheckBox;
import com.google.gwt.user.client.ui.FocusWidget;
import com.google.gwt.user.client.ui.ListBox;
import com.google.gwt.user.client.ui.TextBoxBase;
import java.util.HashMap;
import java.util.Map;
/** Enables a FocusWidget (e.g. a Button) if an edit is detected from any
* registered input widget.
*/
public class OnEditEnabler implements KeyPressHandler, KeyDownHandler,
MouseUpHandler, ChangeHandler, ValueChangeHandler {
private final FocusWidget widget;
private Map<TextBoxBase, String> strings = new HashMap<TextBoxBase, String>();
// The first parameter to the contructors must be the FocusWidget to enable,
// subsequent parameters are widgets to listenTo.
public OnEditEnabler(final FocusWidget w, final TextBoxBase tb) {
this(w);
listenTo(tb);
}
public OnEditEnabler(final FocusWidget w, final ListBox lb) {
this(w);
listenTo(lb);
}
public OnEditEnabler(final FocusWidget w, final CheckBox cb) {
this(w);
listenTo(cb);
}
public OnEditEnabler(final FocusWidget w) {
widget = w;
}
// Register input widgets to be listened to
public void listenTo(final TextBoxBase tb) {
strings.put(tb, tb.getText());
tb.addKeyPressHandler(this);
// Is there another way to capture middle button X11 pastes in browsers
// which do not yet support ONPASTE events (Firefox)?
tb.addMouseUpHandler(this);
// Resetting the "original text" on focus ensures that we are
// up to date with non-user updates of the text (calls to
// setText()...) and also up to date with user changes which
// occured after enabling "widget".
tb.addFocusHandler(new FocusHandler() {
@Override
public void onFocus(FocusEvent event) {
strings.put(tb, tb.getText());
}
});
// CTRL-V Pastes in Chrome seem only detectable via BrowserEvents or
// KeyDownEvents, the latter is better.
tb.addKeyDownHandler(this);
}
public void listenTo(final ListBox lb) {
lb.addChangeHandler(this);
}
public void listenTo(final CheckBox cb) {
cb.addValueChangeHandler(this);
}
// Handlers
@Override
public void onKeyPress(final KeyPressEvent e) {
on(e);
}
@Override
public void onKeyDown(final KeyDownEvent e) {
on(e);
}
@Override
public void onMouseUp(final MouseUpEvent e) {
on(e);
}
@Override
public void onChange(final ChangeEvent e) {
on(e);
}
@Override
public void onValueChange(final ValueChangeEvent e) {
on(e);
}
private void on(final GwtEvent e) {
if (widget.isEnabled() ||
! (e.getSource() instanceof FocusWidget) ||
! ((FocusWidget) e.getSource()).isEnabled() ) {
return;
}
if (e.getSource() instanceof TextBoxBase) {
onTextBoxBase((TextBoxBase) e.getSource());
} else {
// For many widgets, we can assume that a change is an edit. If
// a widget does not work that way, it should be special cased
// above.
widget.setEnabled(true);
}
}
private void onTextBoxBase(final TextBoxBase tb) {
// The text appears to not get updated until the handlers complete.
DeferredCommand.add(new Command() {
@Override
public void execute() {
String orig = strings.get(tb);
if (orig == null) {
orig = "";
}
if (! orig.equals(tb.getText())) {
widget.setEnabled(true);
}
}
});
}
}

71
gerrit-gwtui/src/main/java/com/google/gerrit/client/ui/TextSaveButtonListener.java
View File

@@ -1,71 +0,0 @@
// Copyright (C) 2008 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.client.ui;
import com.google.gwt.event.dom.client.KeyCodes;
import com.google.gwt.event.dom.client.KeyPressEvent;
import com.google.gwt.event.dom.client.KeyPressHandler;
import com.google.gwt.user.client.ui.FocusWidget;
import com.google.gwt.user.client.ui.TextBoxBase;
/** Enables an action (e.g. a Button) if the text box is modified. */
public class TextSaveButtonListener implements KeyPressHandler {
private final FocusWidget descAction;
public TextSaveButtonListener(final FocusWidget action) {
descAction = action;
}
public TextSaveButtonListener(final TextBoxBase text, final FocusWidget action) {
this(action);
text.addKeyPressHandler(this);
}
@Override
public void onKeyPress(final KeyPressEvent e) {
if (descAction.isEnabled()) {
// Do nothing, its already enabled.
} else if (e.isControlKeyDown() || e.isAltKeyDown() || e.isMetaKeyDown()) {
switch (e.getCharCode()) {
case 'v':
case 'x':
on(e);
break;
}
} else {
switch (e.getCharCode()) {
case KeyCodes.KEY_UP:
case KeyCodes.KEY_DOWN:
case KeyCodes.KEY_LEFT:
case KeyCodes.KEY_RIGHT:
case KeyCodes.KEY_HOME:
case KeyCodes.KEY_END:
case KeyCodes.KEY_PAGEUP:
case KeyCodes.KEY_PAGEDOWN:
case KeyCodes.KEY_ALT:
case KeyCodes.KEY_CTRL:
case KeyCodes.KEY_SHIFT:
break;
default:
on(e);
break;
}
}
}
private void on(final KeyPressEvent e) {
descAction.setEnabled(((TextBoxBase) e.getSource()).isEnabled());
}
}