First of all, Johan, thanks a lot to having commited in cvs the
proposal I submitted to the list.
I just have some remarks (that I hope are constructive) :
General ones:
---------------------
- I don't understand why you have been keeping the previous way of
maintaining the "no user input" stuff, by having a String constant of
the form "[-NO-RAW-INPUT-]" and fill FormComponent.rawInput property
with it. I found it more elegant to have a boolean associated with the
rawInput (boolean named modelUpToDate if I remember well) and get rid
of the weird constant. What if the user wants to enter the text
"[-NO-RAW-INPUT-]" in a TextField. Of course there isn't a great
probability, but it hurts my logical way of thinking ...
Was this done for some performance reason ? (I doubt of it)
And more than that : you haven't got rid of the above mentioned
modelUpToDate boolean I've introduced, but it isn't used in the code,
because you've rollbacked my use of this variable in favor of the old
"[-NO-RAW-INPUT-]" style.
If you want to keep the "[-NO-RAW-INPUT-]" approach, this won't
prevent me to sleep tonight, but then I think you should get rid of
the modelUpToDate property.
- I would have been pleased if you had filled the "due-to" section of
the changes.xml doc concerning the 2 proposals I made on the list.
(Form aware things solution, & html escaping fix in
FormComponent.getValue()). It took me time to do this, and this little
reward would have been welcomed (it's not too late to correct this,
though ;-).
Specific ones:
---------------------
(since I've checked what you've commited, because I had some interests
in those) :
- changes.xml:
You wrote:
<action type="add" dev="Johan Compagner">Form will hold the values now
in its own state until everything is validated</action>
I think the following sentence may been better explain the fix :
<action type="add" dev="Johan Compagner" due-to="Laurent
Petit">FormComponent will now hold the values until they are
(successfully) validated</action>
- Form.java:
It seems that in your commit you've mixed the fix that allows
FormComponents to keep the user input until they are successfully
validated, with something else, related to cssId.
I have a remark and a question on this:
- remark: I can see in the code this line :
tag.put("action",
Strings.replaceAll(urlFor(IFormSubmitListener.class), "&", "&"));
This reminds me on the current discussion we have with the possibility
that ComponentTag class could just do the html escaping as a service
for Components developers. If the team accepts this modification, then
it may be possible that this line has to be updated (getting rid of
the Strings.replaceAll() call).
- question: could you enhance the javadoc of the methods
getCssId() and onComponentTagBody() in order to explain the purpose of
the cssId stuff ?
Fiuu, was a long post !
If you read it all, congratulations ;-))
Thanks in advance for your remarks & answers,
cu,
--
Laurent
-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems? Stop! Download the new AJAX search engine that makes
searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_idv37&alloc_id865&op=click
_______________________________________________
Wicket-develop mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/wicket-develop
