Noel,
The code looks great - my comments are inline. Nothing major - most are simple
naming issues.
Looking forward to adding this to the codebase. Once your ICLA is on file, we
can add you as a committer and you can check it in yourself.
Greg
Index: wtk-test/src/pivot/wtk/test/TextInputValidatorTest.java
===================================================================
--- wtk-test/src/pivot/wtk/test/TextInputValidatorTest.java (revision 0)
+++ wtk-test/src/pivot/wtk/test/TextInputValidatorTest.java (revision 0)
@@ -0,0 +1,135 @@
+/*
+ * Copyright (c) 2008 VMware, Inc.
[gkb] You don't need to include the VMware copyright in your files unless you
want to (though your employer may require you to use their own copyright,
depending on who you work for/where you live).
Index: wtk/src/pivot/wtk/TextInput.java
===================================================================
--- wtk/src/pivot/wtk/TextInput.java (revision 755889)
+++ wtk/src/pivot/wtk/TextInput.java (working copy)
@@ -132,7 +139,9 @@
private boolean password = false;
private String prompt = null;
private String textKey = null;
-
+ private TextValidator textValidator = null;
[gkb] Naming - calling this property "validator" would be preferable; see
comment on TextValidator interface name.
+ private boolean isTextValid = true;
[gkb] Naming - call this member "textValid" (we don't use an "is" prefix on
member variables, only getters).
+ public TextValidator getTextValidator() {
+ return this.textValidator;
+ }
[gkb] Naming - call this method getValidator().
+ private void runTextValidation() {
[gkb] Naming - call this method "validateText()" (not critical since it is
private, but we prefer to be as concise as possible in our naming without
losing detail)
+ String s = getText();
+ boolean b = textValidator == null ? true :
textValidator.isValid(s);
+ if (b != this.isTextValid) {
+ this.isTextValid = b;
+ textInputListeners.validChanged(this, isTextValid);
+ repaint();
+ }
+ }
[gkb] Naming - in general, we prefer descriptive variable names over simple
letters; e.g. use "text" here instead of "s". However, for short methods or
where the meaning of the variable is obvious, letters are OK.
Index: wtk/src/pivot/wtk/TextInputListener.java
===================================================================
--- wtk/src/pivot/wtk/TextInputListener.java (revision 755889)
+++ wtk/src/pivot/wtk/TextInputListener.java (working copy)
@@ -68,4 +68,13 @@
+ public void validChanged(TextInput textInput, boolean isTextValid);
[gkb] TextInput should also fire an event when the validator property itself
changes.
Index: wtk/src/pivot/wtk/skin/terra/TerraTextInputSkin.java
===================================================================
--- wtk/src/pivot/wtk/skin/terra/TerraTextInputSkin.java (revision
755889)
+++ wtk/src/pivot/wtk/skin/terra/TerraTextInputSkin.java (working copy)
+
+ public void validChanged(TextInput textInput, boolean isValid) {
+ repaintComponent();
+ }
+
[gkb] Naming - call this "textValidChanged()" (for consistency with the
property name).
+ public void validValueChanged(TextInput textInput, Object validValue) {
+ // No-op
+ }
[gkb] Looks like this is an old name for the "validChanged()" event?
Index: wtk/src/pivot/wtk/text/validation/TextValidator.java
===================================================================
--- wtk/src/pivot/wtk/text/validation/TextValidator.java (revision 0)
+++ wtk/src/pivot/wtk/text/validation/TextValidator.java (revision 0)
@@ -0,0 +1,11 @@
+package pivot.wtk.text.validation;
+
+/**
+ * Validation interface for TextInput widget.
+ *
+ * @author Noel Grandin
+ */
+public interface TextValidator {
+ /** Is the text value valid? */
+ boolean isValid(String text);
+}
[gkb] Nit - "Text" in the interface name is redundant, since the interface is
defined in the pivot.wtk.text.validation package.
