I like your idea of renaming, although I think I will rename the
method in LzNode to be acceptTypeValue and presentTypeValue (because
they take a type argument and convert a value according to that
type). I will take your suggestion to rename the method in components
to acceptValue and presentValue (because, although I permit an
optional type, by default they will use the type declared on the
component to convert the value of the component).
Now that I look at it that way, I see that really _all_ components
should have this acceptValue/presentValue interface, and those
components that are text (basic, simpletext, edittext, etc.) should
use their text as their value. Then there really can be a default
method for applyData/updateData that uses acceptValue/presentValue,
and it is only certain components that have additional behavior on
updateData that will have to override that.
As a result, I am going to revise this change and re-send it to you
for review.
Sorry, but you earned it! :)
On 2008-11-16, at 10:45EST, André Bargull wrote:
Approved.
I wonder whether it makes sense to switch "LzNode#[accept/
present]Value(..)" with lz.basevaluecomponent#[accept/present](..)",
so that we call the general method "accept" and "present" and place
them on LzNode, and use "acceptValue" resp. "presentValue" for
lz.basevaluecomponent.
Some reasons:
1. the new "accept{..}" constraint (if it gets implemented), uses
the LzNode method. If it is "accept()" instead of "acceptValue()",
users will see more easily the connection between the constraint and
the method.
2. "value" (as a term) is used in basevaluecomponent, but not
anywhere in LzNode. So it's more natural to call it "acceptValue()"
if we want to change the "value" attribute on lz.basevaluecomponent.
On 11/16/2008 12:48 AM, P T Withington wrote:
Change 20081115-ptw-x by [EMAIL PROTECTED] on 2008-11-15
18:26:45 EST
in /Users/ptw/OpenLaszlo/trunk
for http://svn.openlaszlo.org/openlaszlo/trunk
Summary: Respond to review comments on r11780 and r11781
Bugs Fixed:
LPP-7339 Can't use LzNode#presentAttribute in a constraint (previous
fix broke DHTML color conversion)
LPP-7340 basevaluecomponent should have a 'type' so you know how to
accept/present it (previous fix broke updateData protocol)
Technical Reviewer: [EMAIL PROTECTED] (pending)
QA Reviewer: [EMAIL PROTECTED] (pending)
Details:
LzUtils, PresentationTypes: move (incorrect) conversion of color
value to string name from LzColorUtils.inttohex to
ColorPresentationType.present
baselist: Remove useless override
baseslider: Use present, not updateData to get the thumb label
baseformitem: use new present API from basevaluecomponent to
compute updateData
basevaluecomponent: Add new API's accept and present which can be
used to set/retrieve the value as a string according to type. Fix
getValue dependencies. Remove incorrect applyData/updateData and
updateData dependencies method. Make sure present uses getValue
to retrieve the value to be presented. Correct present
dependencies method.
Tests:
Andre's test case from LPP-7340, Lou's color example (revised to
use 'present' in place of 'updateData').
Files:
M WEB-INF/lps/lfc/services/LzUtils.lzs
M WEB-INF/lps/lfc/core/PresentationTypes.lzs
M lps/components/base/baselist.lzx
M lps/components/base/baseslider.lzx
M lps/components/base/baseformitem.lzx
M lps/components/base/basevaluecomponent.lzx
Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20081115-ptw-x.tar