Bug ID: JDK-8097928 Add Formatted TextField control

JDK 8
8u40Fixed

Ok, thanks. I probably misunderstood the target. The line between a validation framework and formatted textfield is different for different developers i guess. The features in one will surely affect the possibilities of the other though. 4) Was just to make sure that it's OK (or specified how to and under what circumstances one can) modify the content of the TextField from the format change listener.
19-08-2014
Mikael, this feature is more lower-level, so it isn't made to handle your use-cases at all. It's up to the implementation to handle this. 1+2) To allow intermediate states, the filter just has to accept anything that is a valid or valid intermediate state. The red/yellow flagging of the field is up to the developer. There's no such direct support in TextField. You can listen on value or text to react on the current value or text and either do some "flags" or enable/disable buttons or show some error text or whatever you need to do in case the input is not valid. 3) Mask format will be handled by a specific subclass of TextFormatter, see RT-37785 4) I don't understand your question. When you get a "Change", you should modify the Change directly, not the field. E.g. change the range to 0 - length (means the whole textfield) and set the new text. This will not result in a new Change, so no infinite loop is going to happen.
19-08-2014
Just a very late-to-the-party two cents. Sorry if I'm re-iterating something that has already been talked about but the thread was basically too long to read.. Just ignore me if I'm off. I am surprised that requirements aren't what's first discussed and agreed on. And then, when what the field should do, an API around that is crafted. Maybe this process happens at Oracle? A few important use cases/requirements that might have been forgotten: 1) A format is set, say a telephone number, but no text is entered. I.e. it's empty. Empty is OK format wise, but if there is something there it must follow the phone number format. 2) The transition from empty to a valid telephone number can not happen instantaneously. First, the field cannot be be so stringent that it doesn't allow the transition. Second, there might be need for some "intermediate" state where the current value is not OK but you still don't want to upset the user and red-flag it. For instance, from a UX perspective maybe I want to have the background yellow while I'm typing the number but red if I leave the field with an uncompleted number. Or, I just disable the OK button while the number is entered (better from a UX perspective). 3) Format placeholders (__ __ __) or # like in Swing is not a very popular thing for users IMO. If they are used, a lot of time and effort needs to be put into making them easy to edit. This is a problem with Swing, they are hard to edit because they don't really do what you'd expect all the time regarding overwrite and selecting sub-parts. At least users thing so. For instance what happens if you double-click the year in a YYYY/MM/DD field, is only the YYYY selected or not? What happens when you start tying. Is the fyll MM part selected when year is entered or is the cursor blinking after the last Y? And so on... 4) Sometimes when you get a "Change" you need to change the whole textfield because it's not enough to just modify the Change itself. Make sure that the setting of the whole textfield in the change listener is specified. It's easy to end up in flag/infinite loop hell if one doesn't clearly specify how this should work.
19-08-2014
Thanks for seeing this through. Now for follow on work: * we need a HelloFormattedText example that shows filtering and formatting (either release code or make a JIRA) * we need to modify Ensemble to make use of the new capability (please make a JIRA) * please release any testing code that makes sense (but is not ready for prime time) to the closed repository rather than the team one (no JIRA for this) Can you think of any more actions? If so, please enter follow on JIRA. Thanks.
15-08-2014
Changesets: http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/290274db83f7 http://hg.openjdk.java.net/openjfx/8u-dev/rt/rev/13f33b110a99
15-08-2014
And the changes are minor enough that we don't need another webrev before you push.
14-08-2014
Ok then, address Kevin's concerns and let's release the code.
14-08-2014
commitValue is called from Behavior object, as a response to key press. Having this on TextInputControl is consistent with other methods used from Behavior. To me, it seems more logical on the control, as TextFormatter itself doesn't have anything (current text) to commit and convert to a value.
14-08-2014
One thought: There is API on TextInputControl called commitValue(). Does it make more sense to have the API on TextFormatter? This is where all the API that is concerning the value is currently located, however, the text control is the place were the editing is presently happening. I am ok with leaving the API where it is but would like your thoughts. If we decide to change this (unlikely), we can do this in another JIRA. +1 (on the API and the web rev)
14-08-2014
+1 on the API
14-08-2014
Looks good to me. Just a few nits: FormatterAccessor.java: * Copyright year should just be 2014 TextFormatter.java: * Copyright year should just be 2014 * Typo in @since tag (there is an extra 'T' after 8u40) * (minor) formatting issue: public static final class Change implements Cloneable{ can you add space after 'Cloneable' ? TextInputControl.java: * textFormatter, undoable, redoable properties needs @defaultValue in javadoc (but the javadoc can be done in a later pass).
14-08-2014
http://cr.openjdk.java.net/~msladecek/rt-14000/webrev.10/ Fixed copyrights and tests
14-08-2014
In addition to Steve's comments, I applied the patch (reverting the unrelated changes in Node.java and Scene.java) and I get a whole pile of unit test failures in TextInputControlTest.
13-08-2014
1) FormatterAccessor contains no copyright. 2) TextFormatter contains no copyright.
13-08-2014
The web rev seems to contain bogus changes to Node and Scene. Upload a new one?
13-08-2014
http://cr.openjdk.java.net/~msladecek/rt-14000/webrev.09/
13-08-2014
1) Please change getControlProposedText() to getControlNewText(). It is a shorter name and matches the Javadoc better. 2) TextInputControlBehavior.charInterator is not used. Please delete it. After this, we can do a pass on the Javadoc text but this can be done post-commit.
13-08-2014
http://cr.openjdk.java.net/~msladecek/rt-14000/webrev.08
13-08-2014
Will fix the annotation name and remove the obsolete class. We agreed with Steve to rename PLAIN_STRING_CONVERTER to IDENTITY_STRING_CONVERTER. The webrev should be ready tomorrow.
12-08-2014
How about renaming PLAIN_STRING_CONVERTER to DEFAULT_STRING_CONVERTER?
12-08-2014
The API looks good. Did not run the code yet. Do I need the fix to RT-38273 in order for things to work? I saw the following (annotation name wrong): public TextFormatter(@NamedArg("defaultValue") UnaryOperator<Change> filter) { this(null, null, filter); } Unused class (unless my IDE is lying): private static class NullConverter<V> extends StringConverter<V> { @Override public String toString(V object) { throw new UnsupportedOperationException(); } @Override public V fromString(String string) { return null; } }
12-08-2014
Without FilterTextFormatter : http://cr.openjdk.java.net/~msladecek/rt-14000/webrev.07/
12-08-2014
http://cr.openjdk.java.net/~msladecek/rt-14000/webrev.06 Changed the name of the property to textFormatter and cleaned the unrelated changed. Filed RT-38273 to track the change to FormatStringConverter.
12-08-2014
I just realized that null values won't work as I would expect for TextFormatter with filter only. The value in FilterTextFormatter is also a way how to provide default text (which is needed for example in MaskFormatter which has to specify some initial placeholder text). With null values (of some unspecified type), the default value of TextFilter cannot serve as a default text (String). Of course, MaskFormatter can provide it's own value converter (the same that's currently in FilterTextFormatter), but any other formatter that needs initial text will have to do the same. I still think subclass is the cleanest way to solve this, but if you insist on more compact API, we can use special String<->String value converter internally.
12-08-2014
I spoke with Martin and we have agreed to do 1) and 2). Further, we have decided that we need a web rev that has only the changes that implement formatting. The next web rev will be reviewed for the details (line endings, Javadoc etc.) and when this one is clean, the code can be released.
11-08-2014
In terms of API, we are quite close. In terms of a change set that can be committed, more work is needed. For example, the change set contains references to AWT/Swing. Also there are tons of changes that are unrelated to filter functionality. I think I commented on this before and we just need to decide what we are going to do about it. My vote would be to get rid of all changes that are unrelated and apply them in another patch (or never). Back to API: 1) get/setFormatter() should be get/setTextFormatter() in order to be consistent with other get/set naming 2) Please get rid of the class FilterTextFormatter and fold the functionality into TextFormatter. I modified TextFormatter quickly to check for null where the valueConverter is used rather than set a default one. I added a constructor that took a single filter and no default value. I change the code so that the value is always null and is never set. This means that people will get null pointer exceptions rather than class cast exceptions when they try to access the value. If you are only setting a filter, you do not care about the value converter or the default value so this makes sense to me. If you have any problems or questions coding 2) or something is unclear, please contact me on Skype so that we can resolve things quickly (we can't afford days of turn around as we are a bit behind). Kevin is gone for a day or two and I would like the final web rev to be in place before he gets back. We also need tdo discuss whether the web rev can have unrelated changes (way better without them). Thanks!
11-08-2014
Updated the method names: http://cr.openjdk.java.net/~msladecek/rt-14000/webrev.05/ Note that TextInputControl does not have setAnchor/setCaretPosition, only the properties. For setting anchor and caretPosition, selectRange(anchor, caret) method is used. But I guess it won't hurt if we have both.
01-08-2014
B) TextInputControl.Change (are start / end needed? is the setter needed?) Here is more thinking about the "previous" named methods. I think it is clearer of "Control" is in the name of the methods indicating that they are referring back to state that is either in the control or that might be set in the control. getControl() getControlText() getControlCaretPosition() getControlAnchor() getControlNewText() There are 3 API methods in Change that talk about "text" and we need to be really careful that is it is clear which text we are referring to. So, getText() is the text in the change object, getControlText() is the current text in the control object and getControlNewText() is the new text that will be put in the control object. Kevin liked getControlProposedText() instead of getControlNewText(). It is a longer name but "proposed" might be clearer than "new". The important part is that "Control" has to be in the name so that we know it is "referring back to the underlying control". Please make these changes and ones in my previous B) comment and release a new web rev.
31-07-2014
The names Steve mentioned above seem good to me. The corresponding Previous names should be changed to match (pending the discussion of whether the name and concept of "Previous" is what we want).
31-07-2014
B) TextInputControl.Change (are start / end needed? is the setter needed?) I think there was a misunderstanding here. The discussion was around "selection" and the names used there. All I was asking for were name changes in a couple of methods. Here is the API of Change that makes sense to me to manipulate the state that is kept in the Change object (it is very close to the method naming of http://cr.openjdk.java.net/~msladecek/rt-14000/webrev.03/): public final int getAnchor() public final int getCaretPosition() public final IndexRange getSelection() public final String getText() public final boolean isAdded() public final boolean isContentChange() public final boolean isDeleted() public final boolean isReplaced() public final void setAnchor(int value) public final void setCaretPosition(int value) public void selectRange(int start, int end) public final void setText(String value) Please use this API because it is consistent with names and concepts used in TextInputControl. I need to think a bit more about the "previous" naming / concept that you introduced and will comment on that later. I feel as if there was a misunderstanding in my previous post even though I attempted to be as clear as possible but it's easy to misunderstand. Kevin is this clear to you as well and does this sound good?
31-07-2014
New webrev: http://cr.openjdk.java.net/~msladecek/rt-14000/webrev.04/ B) improved the javadoc, so that it's more clear what is range for. Now the anchor and caret are set using selectRange(). But we still need both getAnchor() and getCaret() as IndexRange needs start < end and it's not clear which one is anchor and which one is caret then. C) Tried to merge FilterTextFormatter into TextFormatter, but it would really require to create it as TextFormatter<String>( ... /* constructor without value converter */ ). Otherwise ClassCastExceptions might occur. Using raw type erases all generics, so filter is changed from UnaryOperator<Change> to UnaryOperator. It seems as a minor difference, but it's very inconvenient to use when it's in one class. D) I decoupled TextFormatter and TextInputControl. As we discussed, this means the Change has to contain some of the information only control has (like old text/ anchor/ caret), so the number of Change methods grew a little. If this is the price for the possibility to use Formatter in any control, I think it's worth it. And they are definitely needed, e.g. I would not be able to implement even my (quite simple) MaskFormatter without this information. The way it's done now, TextFormatter can be used only in our controls as there are package-private fields/methods that can be used only from controls package. If needed, we can adjust the TextFormatter so that it can be used also in 3rd party controls, but that would require additional API. G) removed the method I'll push the updated samples to team repo. Note that there's a sample for error handling (HelloFormattedFieldInForm).
31-07-2014
OK and there's one problem with selection - we won't be able to distinguish between "negative" and "positive" selection. The selection in TextInputControl is normalized selection (start < end), but anchor can be actually grater than caret (e.g. by clicking and dragging to the left). With the API change, the user won't be able to tell if the new selection is (anchor, caret) or (caret, anchor). This is just a slight inconvenience, but I can imagine somebody would need the information when fine tuning his Formatter (like whether the new caret should be left of selection or right of selection depending where the caret was). So it think it would be better to have: public int getAnchor(); public int getCaretPosition(); public void selectRange(anchor, caretPosition);
31-07-2014
What about changing the API of Change so that it covers all the cases separately: public String getAddedText(); public int getAddedIndex(); public IndexRange getDeletedRange(); public String getDeletedText(); public IndexRange getSelection(); pubic String getOriginalControlsText(); // remember we won't have easy access to the control as it can be any of TextInputControl subclasses or Spinner / DatePicker / (RichTextEditor) public String getControlsText(); + the methods isAdded() / isDeleted() / isReplaced() public void addText(int index, String text) public void deleteText(int begin, int to); public void replaceText(int begin, int to, String text); public void selectRange(int anchor, int caret) Calling addText/deleteText/replaceText will clear the previous modification, so calling deleteText + addText is not the same as calling replaceText
31-07-2014
Now I'm confused. So instead of getCaretPosition() and getAnchor() you want getSelection() and selectRange(int anchor, int caretPosition)? BTW, TextInputControl has both caretPositionProperty and anchorProperty, so it matches the API. But getSelection() and selectRange() seems also fine to me... The other "range" we have in change has nothing to do with selection. The change actually describes two separate changes : the text change which has a deleted range and new text and a selection change. The text change works the same way as ObservableList changes: 1) something is added: range start == range end == index of the position where the text was added; getText() returns new text 2) something is removed: range start < range end. The range describes the range of text that was deleted. The text might have been deleted programatically, so it doesn't have to match the selection (which did not change). 3) something is replaced: the combination of above. range start < range end + getText(). Text in the range is removed and a new text of some arbitrary length has been added in that place (range start). Now, when the text is changed by the user, we can be sure where the caret and anchor were at the beginning. 1) anchor == caret == range start == range end 2/3) anchor == range start, caret == range end. This is the current/old state of the control when the change happens, but the change already contains the new caret/anchor position, which is always range start + getText().length(). But none of this applies for changes done through TextInputControl.API. Also, after the change, the user should have full control over both selection and content change range. E.g. you want mask formatter to skip the fixed characters (like dashes, brackets, etc...). When deleting something, you keep the range, replace the text with fixed characters and placeholder character for editable positions, but adjust the caret/anchor so that they are at some editable position, which might be outside of the range. To sum up: I think your assumptions work only when the field is edited by the user and only on the input of the filter. The output should be more flexible, that's why we need both "ranges". We can have either get/set anchor + get/set caret or selectRange + getSelection. But I can't get rid of setDeletedRAnge() etc.. We may have a better name for this like, setTextChangeRange or setChangeRange. I'll try to make some samples that will (hopefully) make this more clear.
31-07-2014
B) TextInputControl.Change (are start / end needed? is the setter needed?) I wrote some example code and I now understand the behaviour. When a change happens, the filter is called with the selection, the caret and the anchor. The caret and anchor have the value of either the start or end of the selection. The change applies to the selection. If the selection is changed in the change object to reference some other place in the control, the operation will happen in the new selection location. Finally, after the operation takes place at the new selection location, the caret and anchor are moved to the caret and anchor location specified in the change object. Here is some example code that shows the behaviour: public class RangeExample extends Application { public void start(Stage stage) throws Exception { VBox root = new VBox(); TextArea editor = new TextArea(); editor.setText("Hello There"); editor.selectRange(1, 4); editor.setFormatter(new TextInputControl.Formatter ( (UnaryOperator<TextInputControl.Change>) (e) -> { System.out.println( "BEFORE anchor=" + editor.getAnchor() + ", caret=" + editor.getCaretPosition() + ", selection=" + editor.getSelection() ); e.setRange(6, 9); e.setAnchor(1); e.setCaretPosition(2); //e.setAnchor(6); //e.setCaretPosition(9); Platform.runLater(() -> { System.out.println( "AFTER anchor=" + editor.getAnchor() + ", caret=" + editor.getCaretPosition() + ", selection=" + editor.getSelection() ); }); editor.setFormatter(null); return e; }) ); editor.replaceSelection("zz"); root.getChildren().addAll(editor, new Text("What do anchor, caret, start and end mean?")); stage.setScene(new Scene(root)); stage.show(); } public static void main(String[] args) { launch(args); } } I talked to Kevin and Jonathan about this and we agreed that the current behaviour is a bit confusing but it is flexible and allows people to get pretty much any behaviour they want (ie. I type here, text over there changes etc.). The confusion occurs because there are two concepts of "selection" (the selection and the caret/anchor) and it might be possible to have only the selection or only the caret/anchor. For now, we will keep both concepts, however, the Change object selection API needs to match the names from the text control. In Richard's original API, there were the names "New" for caret/anchor and he used text control selection API. Now we know why, however, we don't want to put back the "New" names at this time. RESOLUTION: Get rid of setDeletedRange() etc. and use selection API names from the text control (discussed with Kevin and Jonathan). public final IndexRange getSelection() public void selectRange(int start, int end) I am not violently opposed to getSelectionStart() and getSelectionEnd() but if we add them to Change, they should also be in the edit control so the API's match. Whew!
30-07-2014
My summary from meeting: A) Event vs lambda Events are dropped, we'll go with lambdas B) TextInputControl.Change (are start / end needed? is the setter needed?) Could improve documentation to make start / end clearer Steve wants the 'deleted' name from the API removed Steve would like an example of the API being used C) How useful is the Formatter.valueConverter? Does it offer performance or functional benefits? Martin thinks it would be difficult to use the new API without valueConverter Kevin would like FilterFormatter merged into the Formatter class - the only saving is the saving of typing generic types. Martin can investigate. D) Rename Formatter class to something else? Filter...? Formatter feels a little odd, but it is hard to think of a better name. Formatter might be an OK name if we can grow the API in the future to include the expected functionality with subclasses, such as MaskFormatter. Martin should spend some time investigating the tight-coupling between TextInputControl and Formatter, and how that might play into the rich text editor case - one option is to extract out Formatter from TextInputControl into its own top-level class named TextFormatter. E) Could we get rid of Formatter and move the filter into TextInputControl as a property? Nope! F) How would people build formatters as an external library? Can we do what is shown in http://www.jidesoft.com/blog/2013/06/06/jidefx-beta-release/ (in future releases)? Martin should run the jide examples, and compare the implementations of formatters from Jide with our implementations - are they roughly the same (or better, in terms of complexity) to implement compared with Jide? Could implement some formatters based on jide, as we have a jira issue to include formatters. This should happen after we get the existing code into the repo - although it may inform issues in our API that will require changes. G) isCaretOrAnchorChange() should either be removed and added into example code, or split into two separate methods. For now take it out and see how it feels. Extra: Need to understand better how classes like Change refer to controls, so that in the future we can possibly use the same implementation for RichTextEditor, etc.
30-07-2014
G) Change.isCaretOrAchorChange() seems like a strange API name (it has "Or" the name, there is no "isCaretChange()). Also, it is a helper. Is it needed?
30-07-2014
One more thing. I noticed the following public API with no "@since JavaFX 8u40" (and for the undoable/redoable properties, no javadoc of any kind). public final boolean isUndoable() public final ReadOnlyBooleanProperty undoableProperty() public final boolean isRedoable() public final ReadOnlyBooleanProperty redoableProperty() public final void undo() public final void redo() public final void commitValue() public final void cancelEdit() Do you intend these to be part of the public API? I assume yes, so they will need javadoc.
30-07-2014
Hi Martin, I had a quick chat with Kevin and Jonathan to get their thoughts about the API as it stands right now. We had a few topics that we would like to go over with you over the phone (hopefully resolving things faster than using JIRA in different time zones). Here is the list: A) Event versus Lambda (everyone leaning heavily towards lambda but will quickly summarize) B) TextInputControl.Change (are start / end needed? is the setter needed?) C) How useful is the Formatter.valueConverter? Does it offer performance or functional benefits? D) Rename Formatter class to something else? Filter...? E) Could we get rid of Formatter and move the filter into TextInputControl as a property? F) How would people build formatters as an external library? Can we do what is shown in http://www.jidesoft.com/blog/2013/06/06/jidefx-beta-release/ (in future releases)? Steve
29-07-2014
I made the modifications with Formatter and also others that were discussed: http://cr.openjdk.java.net/~msladecek/rt-14000/webrev.03/
17-07-2014
3) Formatter: Filter Return Value (explore using an event) There is one aspect of event handlers I don't like: Any parent can mess up with events with event filters. Even if it's a TextInputControl specific event, it always goes through the window, scene and every parent (subscene), before the event handlers of the controls are executed (but only if the event is not filtered by some parent). This might be a problem with RichTextEditor in case it would include some other TextInputControl (like font chooser), the commit event would go first through the RichTextEditor. I think it would be better to introduce support to FXML for filters (even if it's specifically just for the filteres) rather then limit ourselves with the current FXML functionality set.
16-07-2014
10) Value Converter / Filter: need a commit callback? Basically, listening for a value change is listening for a commit. This is true only when valueConverter is set, but since the value is not used when valueConverter is null, I was thinking whether we should set identical string on commit as a value. Anyway, with value converter, commit event would be redundant. If we always want to have some value, it would be best if we'd split Formatter into two classes and make it abstract: abstract Formatter + non-public constructor ValueFormatter<T> extends Formatter<T> with constructors: ValueFormatter(UnaryOperator<Change> filter, StringConverter<V> valueConverter, V value); ValueFormatter(StringConverter<V> valueConverter, V value); FilterFormatter extends Formatter<String> and it's constructors ( FilterFormatter(UnaryOperator<Change> filter, String value); FilterFormatter(UnaryOperator<Change> filter); With this split, we can also avoid default text, which will now be simply a default value.
16-07-2014
11) Change: How does Change.setRange(start, end) work? Should it be removed? setRange is not about selection range, but about the range of text being deleted. I should rename it to something like setDeletedRange() to avoid confusion.
16-07-2014
3) Formatter: Filter Return Value (explore using an event) I have made a very quick example that uses an event rather than a lambda to do filtering. First off, I don't believe that a self contained application would normally use lambdas to combine filters. Instead, they would code a single filter that contained the logic of each lambda. This means that having the filter be a lambda is nice for typing etc. but the fact that is it is a lambda is not a huge benefit to the application. One reason you might want to combine filters would be if you are given a bunch of pre-existing code and you want to tweak it. Rather than querying the current filter, assigning a new filter and use andThen() or other lambda operations, people could use the event mechanism of FX. To run A-or-B, an application would hook at filter and if A was to run, it would consume the event. To run A-and-B, an application would add an event handler. One advantage of event handlers is that they are FXML friendly. A disadvantage is that this would decouple the formatter from the event handler, however, I am wondering whether the standard set of Formatters we want to add to FX could just be code that listened for events. The other thing about having an event is that the event and the handlers might be able to be used for the RichTextControl, when we write it. I am just exploring the idea of an event versus a lambda and the chaining that it both imply. The event is the standard mechanism in FX to react when something happens and has well known behaviour that applications can rely on. So, if filtering became an event, what would/could happen to formatting? Is the formatter class needed at all considering that it is now just a value converter, default text and a value property? If this class were to go, then application code would be responsible for converting the value of the text field into an object and could handle error cases directly (i.e. "red error text").
15-07-2014
BTW, when you respond with numbered text, please keep the title along with the number to keep context. Thanks!
15-07-2014
11) Change: How does Change.setRange(start, end) work? Should it be removed? When does it make sense to set all 4 of caret, anchor, start and end? Doesn't the start and the end always have to be equal to either the caret or the anchor? Should we get rid of the ability to set start and end from Change? public class Bug extends Application { public void start(Stage stage) throws Exception { VBox root = new VBox(); TextArea editor = new TextArea(); editor.setFormatter(new TextInputControl.Formatter ( (UnaryOperator<TextInputControl.Change>) (e) -> { if (e.getText().equals("A")) { e.setText("FRED"); //THE RIGHT WAY? //e.setAnchor(0); //e.setCaretPosition(4); //THE WRONG WAY? Should setRange() be removed from the API? try { e.setRange(0, 4); } catch (Throwable ex) { System.err.println("Why is setRange(0, 4) an error but the same thing in runLater() valid?"); } //A STUPID WORK AROUND THAT ILLUSTRATES THE POINT Platform.runLater(() -> { editor.selectRange(0, 4); }); } return e; }) ); root.getChildren().addAll(editor, new Text("Type \"A\" to see \"FRED\" selected")); stage.setScene(new Scene(root)); stage.show(); } public static void main(String[] args) { launch(args); } }
15-07-2014
10) Value Converter / Filter: need a commit callback? Do we need a commit callback or equivalent functionality? I have seen this in other toolkits and also a detail in an event that says whether the commit happened because of focus lost or activate. Currently, when the commit fails, we put back the default value, correct? Does our API support the case where, there is a dialog that has a text field, while the user types, "red error text" is displayed within the dialog and the 'ok' button is grayed. The user can tab around and fill in other areas of the dialog but cannot exit the dialog until valid text is entered in the text field. This is a common case and we should code an example of it.
15-07-2014
8) Change: The booleans (isAdded(), isAppended() etc.) I agree that we need to know the difference between a caret change. If we leave these method, then we should consider recoding them in terms of start, end etc. so that they are up to date when the values of start, end etc. are changed. For example, it is no longer an add when start, end etc. are changed so that start !=end yet the boolean is not updated. I understand that the booleans could refer to the initial operation, however, if we are thinking that the change object captures all the state of the change, then having the booleans not match start,end etc. breaks chaining of filters, no?
15-07-2014
9) With focus listeners it would be tricky - you need to replace the formatter (converter) just after the value was commited and transformed to a value using the current converter, then replace it in order to convert the value to the new text. So it's doable, but we would need something like onAfterCommit event. Anyway, I'll remove the editConverter from the API as it can be added later. 8) We need at least to distinguish between caret/anchor change and text change. That could be either a boolean or enum. It's not necessary to have booleans that describe the different kinds of text change as it can be computed from the change data ( non empty text means added, non empty range means deleted). It's just convenient if somebody want to optimize the code or improve it's readability. The code might be much simpler when only deleting a text comparing to the case where the text is replaced by some other text. It's also similar to the way ObservableList changes work. 10) Value converters are StringConverters and the javadoc for this class does not explicitely state how to reject a conversion (null or exception?), but it seems that the implementations use exceptions to do this, so when a StringConverter throws an Exception, the commit is not done. 3) There's another problem with A-or-B, if A cancels the object (w/o modifying it) and it was not a clone, how could B un-cancel it and modify it further? This is actually the one thing I don't like about this proposal and it's the duality of Formatter. It's either a filter alone (where value has no use) or a value converter (with optional filter). For the case where value converter and filter are used together, it's better to have them bundled in one Formatter (so there can be one specialized Formatter implementation with everything included) and chaining makes no sense. When using filter alone, the value is superfluous, but the chaining would be very convenient. If we could do setFormatter(makeUpper.then(replaceA)) the creation of a filter would be very convenient, providing there's some nice set of pre-defined filters. One solution for this would be more separation, like a Formatter<T> that contains a Filter and a Converter<T> and would have constructors like this Formatter(Filter, Converter<T>), Formatter(Filter), Formatter(Converter<T>). Filter would contain the UnaryOperator (and optionally a default value) and would be chain-able (with methods then & or). Converter would encapsulate the value (means one extra step to value, sigh) and a StringConverter.
14-07-2014
3) Formatter: Filter Return Value (chaining filters) First off, I don't see that returning null or the Change object is harmful, it is just not what I expected in terms of API. If it is really useful, then we should do it. There are two cases that it helps: A-or-B or A-then-B. In the case of A-or-B, it is unlikely that A would set some fields in the Change object, then cancel. If A did that, then it would be a programming error. In the case of A-then-B, a clone() of the Change object is not needed. The same one can be passed to both filters. Why might an application want to compose filters? One reason is that a library of formatters/filters was provided and it needs to be tweaked. In this case, you would query the existing formatter/filter and set an new one that chains the previous one. public class HelloFormattedTextFilters extends Application { @Override public void start(Stage stage) throws Exception { VBox root = new VBox(); TextArea editor = new TextArea(); TextInputControl.Formatter makeUpper = new TextInputControl.Formatter( (UnaryOperator<TextInputControl.Change>) (e) -> { if (e == null) return null; e.setText(e.getText().toUpperCase()); return e; } ); TextInputControl.Formatter replaceA = new TextInputControl.Formatter( (UnaryOperator<TextInputControl.Change>) (e) -> { if (e == null) return null; e.setText(e.getText().replace("A", "Fred")); return e; } ); editor.setFormatter(new TextInputControl.Formatter ( (UnaryOperator<TextInputControl.Change>) (e) -> { return (Change) ((makeUpper.getFilter().andThen(replaceA.getFilter())).apply(e)); }) ); root.getChildren().addAll(editor, new Text("A-then-B")); stage.setScene(new Scene(root)); stage.show(); } public static void main(String[] args) { launch(args); } } It's quite a mess (I'm betting you could write it more cleanly). Essentially, chaining is not something that normal application code should do. Instead, they should just write all of their filter code in one place. Do you have a lambda example for the A-or-B case that uses new JDK8 API? Where am I going with this? I am exploring whether extending behaviour is easier by chaining lambdas or whether we should consider a TextChange event instead of a filter. This also might be overkill but it would mean that filtering could be done easily from FXML and that tunnelling and bubbling could be used. On the down side, it would separate filtering from Formatting. Finally, there is talk of a RichTextEditor and this will need formatting/filtering. If it is a subclass of TextInputControl, then all is cool. Thoughts?
11-07-2014
8) Change: The booleans (isAdded(), isAppended() etc.) I saw that you got rid of some of them and not all. I'm wondering if we have a problem here. Should not all changes where text has been modified have a boolean associated with them? Perhaps this is why the original code had so many. Will every change have one of these booleans always "true"? I am still not convinced that it is safe / wise to make use of these booleans when evaluating a change. Could you please comment your sample code to explain why special processing is needed based on the kind of change. Also, aren't isAdded(), isDeleted() and isReplaced() easily computable from the current selection in the control and the proposed selection and new text in the change? If text is being added, then the new selection caret == anchor. If text is being deleted, then caret != anchor and new text is empty. If text is being replaced, caret != anchor and new text is not empty. Is my understanding of these boolean flawed?
11-07-2014
10) Value Converter / Filter: need a commit callback? How does the value converter reject the change when focus is lost or ENTER for single line controls is pressed? I have seen other toolkits where there is a commit() API and a commit callback that has the reason the commit was called (focus lost or ENTER or unknown) so that the application can react accordingly. We don't necessarily need to add such an API, but I'm wondering how we do equivalent processing with the API we have.
11-07-2014
4) Change: Naming RESOLUTION: getNewControlText() should be getControlText()
11-07-2014
9) The Edit Converter: Is it needed / overkill? I'm betting the same functionality could be coded with focus gained listeners. We should investigate this using example code. RESOLUTION: Get rid of it for now. Even if we can't get the same behaviour using existing API, we can always add this capability later.
11-07-2014
The value is currently commited exactly as you say, on action (ENTER) and focus lost. ValueConverter does not have access to the control, so it can't decide based on the state. I understand that the edit converter is not going to be used in most of the Formatter instances, so we can just remove it for now and possibly add it later if it's requested by our users.
11-07-2014
9) The Edit Converter: Is it needed / overkill? I think we should get rid of it. It seems that it is adding confusion for not much value. It seems that the value converter could be used based on the state of the control. Should there also be a discussion about when values are committed? Probably on focus lost and when the Action event is fired for single line text.
10-07-2014
The edit converter is supposed to work in the following way: 1) the control is focused - the edit converter is used to convert the current value to a more edit-friendly form (like a formatted phone number with dashes and brackets to plain number) 2) while typing, only filter is being applied. 3) When commited (focus lost), the edit converter is used again to create value. Then the value converter is used to convert value back to the (better formatted) text. The dualism of converters is actually borrowed from JFormattedTextField, although it's not explicit there. It has a factory that returns new Formatter based on the current state of the field, which does not need to be focused/unfocused. Anyway, if we decide not to have edit converter in 8u40, it can be added later.
10-07-2014
1) Formatter: Constructors Thanks for the tuning and the FXML update. I am still not 100% convinced that we should not be using properties but it is mostly the convenience of set/get I am interested in versus enabling people to listen for property changes and the complexity setting one property independent of another might cause to the implementation. I see we are down to one error case where the edit converter is not null and the value converter is null (will dig into that in a separate comment). I see a try {} catch {Exception (e) {}} in line 206. If you ever swallow an exception, you should put a comment that explains why this is ok. Not quite done with the constructor thing but let's leave it for now.
10-07-2014
9) The Edit Converter: Is it needed / overkill? I guess I don't understand the edit converter. Is this accurate: "While the user is typing, the edit converter will be running that will be converting whatever is being typed into Number (for example) and when the user hits return or loses focus, the value converter that will convert whatever text is in the control at that point also to a Number". a) Why are two different converters needed? b) Would it be an error for the two different converters to convert to different types? (yes) I'm wondering when you would ever want two different converters? Is having them adding more complexity to the API when the same result could be achieved some other way. Can you give a simple example of why you might need it (I know you have some code that uses it but that code has no comments). Is it so that while you are typing, you can ask for the value of the formatter?
10-07-2014
Next iteration: http://cr.openjdk.java.net/~msladecek/rt-14000/webrev.02
10-07-2014
Kevin, the IllegalStateException is being thrown if this is violated, see Formatter#bindToControl. Will update the @since tags. Thanks
10-07-2014
Ad Steve comments: 1) The reason why I use constructors is that I want to avoid properties in Formatter. This would complicate the implementation a bit as all of the properties of the current filter would need to be followed. So I feel it's better to have Formatter as a non-mutable entity (with the exception of the value). Also since valueConverter and filter need to cooperate with each other (output from formatter should pass the filter), it might be problem to switch both through properties without some awkward mid-state. But the set of constructors needs to be tuned a bit, so that you don't need to cast nulls and such. Constructors are also FXML friendly, if we provide @NamedArg annotation for the parameters. From FXML perspective, there's no difference. 2) Already used by MaskedFormatter. It does not need a value, but wants to enforce some format with some pre-defined Strings that are always present + some blank spaces. The filter needs some initial state to work correctly, but I agree that I probably over-engineered it when I used lambda instead of plain String here. 3) Consider use-case where you want to chain filters (I actually use that at least in one of my samples). There are 2 simple ways to chain them A-or-B or A-then-B. In the first case, if A fails, B should be used. In order to do this, you need to provide a clone to A, so if it fails, we can pass the original change object to B. With flags (cancel or doit), we'd have to restore the change object to some state before A in order to pass it to B and the cancel or doit flags would need to be cleanable (which is strange). If A passes, you can just return whatever A returned. In case of A-then-B, the doit flag would not work (you cannot double-confirm something). So in general, the current way is good for cases where you need to get back to the original modification in case of some troubles. 4) OK 5) See #3 6) IndexRange is a class already used in TextInputControl, so I guess Richard included it just for convenience. We can just remove it now and add it back on request. 7) Will do. 8) Added, deleted and maybe replaced are useful. I can't think of any use-case for prepended, appended and inserted and I don't see any of them used in the samples from Richard in RT-30881, so I don't know why he included them. I'll remove them.
10-07-2014
In addition to the comments Steve wrote, I have a couple other points: 1) In the formatter class you state: "a single Formatter instance can be used only in one TextInputControl at a time." Perhaps this could be enforced by checking and throwing an IllegalStateException if violated? 2) You need "@since JavaFX 8u40" for the two new nested classes. Also, you need to change "@since JavaFX 8.0u40" on the new formatter property to "@since JavaFX 8u40".
09-07-2014
Hi Martin, In order to explore the API further, I wrote a simple undo/redo manager. I will clean up the code at some point and add it as an actual example because it exposed a few bugs (might also be bugs in the example code too). In any case, a complete filter API should powerful enough to support undo/redo capability that is external to the text control. If it can't, the API is not good enough (it can). I have a few comments: 1) Formatter: Constructors I found using formatter to be a bit verbose/awkward. This was mainly due to the use of constructors. I wanted to only set a filter but the constructors made me pass in a StringConverter. I found out it could null later if I cast to the correct kind of null but initially, I provided a StringConverter that converted back and forth to a String. If I am just writing a filter, what do I need a string convertor for? I noticed that the constructors enforced some rules (can't have a null this and non-null that etc.). My feeling is that it should be legal for all fields to be null (this would essentially be a null-formatter that did nothing when installed). So, I'd like to replace the constructors with set/get methods (likely properties in order to match valueProperty()). I feel that get/set methods that were properties would fix the problem thatI had where I had a big long constructor expression with embedded lambdas in it (yes, I could have used temporary variables) and make things more usable. It might also help with FXML. 2) Formatter: Default Value Why is this needed? Given that it is necessary, why is it's value provided by a lambda? My feeling is that this should simply be a string or an instance of V (the class of the set/getValue()). I am not that concerned that it be an instance of V, but my feeling it should just be a String. 3) Formatter: Filter Return Value The filter takes a Change and returns a Change. If the Change that is returned is null, then the operating doesn't happen. This is a bit confusing. The normal mode of operation is to modify the Change that was passed in and return that. This is what 99.9% of the people using the API will be doing. It is possible to clone() the change (more on this later) and return a new instance but I don't see any reason why you would need do this. It seems more natural that the method should return void and the only way that you can change the text that will appear in the controls is by manipulating the fields of the control. In FX, we have used cancel() and I have seen "doit" fields in other toolkits. 4) Change: Naming I see that we use "New" in various places. I think we can simply delete "New" from the method names and these names will match the ones in the text control. So we would get setAnchor() etc. 5) Change: Get rid of clone() My view is that we should not allow people to poof up a different instance of a Change and return that instead of the one that we passed in to the filter. If we need this capability, we can always add it back. 6) Change: Get rid of IndexedRange API I don't see that this API is needed if we have get/set for the start and end. Ether we only support range based setting or we only support get/set start and end. If this turns out to be a problem, we can always add these methods back later. 7) Change: Need a nice toString() It was pretty hard to debug my test code without a print that told me what was going on. I needed to see at least the start and end and new text. 8) Change: The booleans (isAdded(), isAppended() etc.) I'm not sure how useful these are. I see you make use of them in MaskFormatter. To me, it should not matter how the text changed happened and code that reacts to it should "just work the same way". Perhaps you can explain why these booleans make MaskFormatter code easier to write (or how the code would be incorrect without them). Using them seems to be begging for errors in application code based on the mode of text replacement etc. That's it for now, Steve
09-07-2014
Attaching some samples (rt-14000_samples_01.zip) that work with the latest patch (webrev.01). The MaskFormatter is the most complex of them (although it doesn't use value and value converter). It's similar to MaskFormatter from swing.
08-07-2014
I updated the API so that caret/anchor changes can be intercepted too. This requires only one additional method in "Change" (previously ContentChange): public final boolean isCaretChange(); Here's the webrev: http://cr.openjdk.java.net/~msladecek/rt-14000/webrev.01/
08-07-2014
Yes, this should work. formatter is a property, so you can set any Formatter object to it. We just have to make sure that all our formatter implementations are fxml friendly (when working on RT-37785)
07-07-2014
Will this syntax work today? It seems "yes" provided that formatter is a property which it is. We will need to code a trivial example that shows this working.
07-07-2014
I expect most of the times, the formatter will be set using some subclass, like this: <TextField> <formatter> <MaskedFormatter mask="UUU-UUU"/> </formatter> </TextField> I think a pure formatter class will be rarely so simple it can be reasonably set through FXML.
07-07-2014
Before I forget, the current API is not FXML friendly. If we were to express the filter / formatting behaviour as an event, then it could be programmed from FXML. As it is, this new functionality can't be used. Is this correct? I believe that expressing the filter / formatting as an event should not be hard, but let's discuses the possible solutions.
07-07-2014
I was also thinking about passing caret & anchor changes throught the filter. They can be already changed by the filter and possibility to intercept a caret movement would allow implementation to skip some parts of the text when moving caret. It's very similar to what NavigationFilter in JFormattedTextField is for.
02-07-2014
I'm going to work on some samples, I also plan to include a few simple samples in the Formatter javadoc.
02-07-2014
A new proposal in a webrev format: http://cr.openjdk.java.net/~msladecek/rt-14000/webrev.00/ The Formatter class that combines the functionality of formatted text field and content filter (RT-30881). We talked with Steve and Kevin that we should file another JIRA for undo/redo methods that were part of the original patch from Richard Bair and make them private for now. I found that it's not possible however. The reason is that we need to call them from behaviour and skin. Since these are different packages, it's not possible to do "hide" them without introducing yet another impl_* method, which we want to avoid. This seems like a common pattern in controls. The TextInputControl itself has many public methods that (I think) were created mostly for calls from the skin/behavior, like deleteNextChar/deletePreviousChar etc...
02-07-2014
Martin, I admit I don't yet have a full grasp of the proposed API, but I think generifying TextInputControl (or TextField) is ok, assuming it can be done in a backwards compatible way. I think the fact that generics are not always relevant is not a major concern - the main point is that generics will help in the case in which they are intended (that is, for formatted text input).
26-06-2014
There's a bug(?) in JDK that prevents us from adding a generic parameter to the class: https://bugs.openjdk.java.net/browse/JDK-8048225
26-06-2014
On the other hand, having a formatter property would allow us to merge content filter into the formatter. This way, the setup of formatted text field would require only this single property vs. formatter + contentfilter. TextField field = new TextField(); Formatter<Number> numberFormatter = // the formatter has both StringConverter<Number> and ContentFilter, or possibly only one of them // Again, field.getFormatter().valueProperty() is of type ObjectProperty<?>, so we need to keep the reference to Formatter<Number> to get the right generic type The problem with the generic type can be solved by having typed TextInputControl<T>. This is backward compatible, but might be confusing since it's meaningful only for TextInputControl with a Formatter. Most of TextFields/TextAreas won't need a typed parameter.
24-06-2014
Jonathan raised a concern that introducing a new class might complicate the integration of the API into ComboBox, DatePicker or Spinner. All of these have "editor" property which returns TextField (and obviously cannot be changed to FormattedTextField). Although FormattedTextField is a TextField and it's additional properties will be probably used/bound internally, we might not support all the use-cases w/o a cast (I don't know exactly what the use-cases might be since I don't fully understand the purpose of editor property). I we were to incorporate formatting into TextField (or even TextInputControl) directly, we can't use generics, so the property for some new Formatter would look like: ObjectProperty<Formatter<?>> formatterProperty(); So in order to get the right value, the Formatter has to be stored in another variable with the right generic type, like this: TextField field = new TextField(); Formatter<Number> numberFormatter = // some initialization with StringConverter<Number> field.setFormatter(numberFormatter); numberFormatter.valueProperty().addListener(// a listener with Number); vs. FormattedTextField<Number> field = new FormattedTextField<>(someStringConverter); field.valueProperty().addListener(...)
24-06-2014
Attaching some samples for the new API (formattedtextfield.tar.gz). * HelloPhoneField - FormattedTextField for phone numbers * HelloFormattedTextField - simple percentage FormattedTextField * FormattedTextFieldDemo - The JFormattedTextField sample from here http://docs.oracle.com/javase/tutorial/uiswing/components/formattedtextfield.html in FX * MaskContentFilter - content filter similar to javafx.swing.text.MaskFormatter - not necessarily for FormattedTextField, no need to use values
11-06-2014
RT-30881 has a formatted text field prototype attached as a patch. The formatted text field functionality relies on the content filter that is the subject of RT-30881.
11-06-2014
This would be a great feature in order to consider adopting JavaFX as the client platform for standard business applications. This is the kind of applications I am primarily interested in. Visual effects, animations, etc... are great, but as a standard business application developer I am more interested in forms displaying currencies, dates and some other data types. Besides data conversion and representation, I also miss "out of the box" complete solutions for POJO binding and validation But just talking about data conversion and representation, having the FX counterpart of Swing JFormattedTextField, even improving this one, would be great and would facilitate (in my opinion) the acceptance of JavaFX.
04-02-2014
Just so you guys know, JIDE open source the MaskTextField and FormattedTextField at https://github.com/jidesoft/jidefx-oss. If you are interested in using a Formatted TextField before JavaFX has its own, you can give it a try. You can see some screenshots of it at http://www.jidesoft.com/blog/2013/06/10/highlights-of-the-jidefx-beta-release-1-of-3/. (Hope you are okay with such ads-like post since the controls are free and open sourced anyway)
08-07-2013
Putting this in the Lombard backlog for consideration.
26-06-2012
No new target release is known. Whether or not it can be done in Lombard depends on the other Lombard priorities.
13-03-2012
very sad news.
21-02-2012
Per discussions with Brian and Ajay, we decided that this cannot be done in 2.2 and hence request to CCR this out of 2.2. Need to have a target release in place before this CCR can be approved.
20-02-2012
No - development hasn't started on this feature yet and JavaFX 2.1 is well past feature-freeze. It is part of the JavaFX 2.2 release cycle.
03-02-2012
will we have a beta version of this control in 2.1? =)
03-02-2012
Implements A360 Feature http://oracleplan.oracle.com/gotoEntity?entityType=FEATURE&entityId=751261
31-01-2012
Either we allow TextField to have an input mask (my preference), or we add a new text field which has an input mask. Either way, we do this in the next major release.
26-08-2011
hello, could look with love this feature, it is impractical to use components in JavaFX masked (and basically almost any software with data entry using masks)
24-06-2011
Outher sample: in JFormattedTextField String mask = ##/##/####; jftt.setFormatterFactory(new DefaultFormatterFactory(new EmptyMaskFormatter(mask)));
06-06-2011
As in JFormattedField I set formats for numbers, dates, and strings. (eg date - dd/mm/yyyy in JFormattedField __/__/____).
06-06-2011
Can you please expand upon what you're requesting - it isn't clear to me.
04-06-2011

Blocks :	JDK-8090453 - Provide some useful TextInputControl.Formatter implementations
Duplicate :	JDK-8102697 - Add support for formatted number fields
Duplicate :	JDK-8098032 - Add contentFilter property to TextInputControl
Relates :	JDK-8096208 - FormatStringConverter should convert only full Strings
Relates :	JDK-8096719 - [Ensemble] Add TextField with formatter sample to Ensemble
Relates :	JDK-8090341 - Add protected createTextField method to ComboBox and DatePicker
Relates :	JDK-8098032 - Add contentFilter property to TextInputControl
Relates :	JDK-8102248 - Virtual Keyboard: Input Method Constraints
Relates :	JDK-8089950 - Severe ordering of TextInputControl events
Relates :	JDK-8091033 - [RichTextEditor] Allow rich text in text fields
Relates :	JDK-8090938 - [FXML] Allow method references to java.util.function.* classes (or generally, a SAM)
Relates :	JDK-8092281 - Datatype based Input control - like Integer Input Control
Relates :	JDK-8098302 - Add Spinner Control