Hi Jonathan,
> Please see and review the attached patch.
+ In your current patch, both the toolkit controls and the form controls
declare a GroupName property, which leads to ambiguities at runtime.
(A non-product build will report those ambiguities - that's why I
always advertise non-product builds for daily development, it helps
finding errors before they manifest themself as bugs.)
You need to remove the declaration of the GroupName property from
ORadioButtonModel::describeFixedProperties.
Instead, add a call
startAggregatePropertyListening( PROPERTY_GROUP_NAME )
to the ORadioButtonModel's ctors, and add code to the
ORadioButtonModel::_propertyChanged method, which is called when
the GroupName property of the aggregate (which is the toolkit's radio
button model) changes.
Which means you must outsource the code in
setFastPropertyValue_NoBroadcast, which initializes |this|'
CONTROLSOURCE property with the value of the same property of another
sibling of the same (new) group. This is to be able to call it from
both setFastPropertyValue_NoBroadcast and _propertyChanged.
Note that the current version of this code is wrong: It actually
searchs for a sibling with the same Name or GroupName, depending on
which property of |this| just changed. Instead, it must search for a
sibling which is in the same group as |this|.
Imagine for instance a document with 3 radio buttons X/A, Y/B, Z/B
(Name/GroupName) where the name of the first is set to Y => it would
copy the CONTROLSOURCE value of the second to itself, which is wrong,
since both are not in the same group.
- in toolkit/source/controls/dialogcontrol.cxx:
const ::rtl::OUString GROUP_NAME (::rtl::OUString::createFromAscii(
"GroupName" ) );
Just a side note here: Those should usually be written as
const ::rtl::OUString GROUP_NAME ( RTL_CONSTASCII_USTRINGPARAM(
"GroupName" ) );
, as this is more efficient. It doesn't really matter here, it's just
a question of being used to it when it actually does matter.
- UnoControlDialogModel::AddRadioButtonToGroup
- pGroups is superfluous
+ the method name is misleading - it suggests the radio button is
actually added, which is not the case.
- pNamedGroups and pCurGroup should be reference parameters, not
pointers, since they cannot be NULL
- the below code in implUpdateGroupStructure
if ( (( nThisModelStep == nCurrentGroupStep )
|| ( 0 == nThisModelStep ))
&& sGroupName == sCurGroup
)
has a weird indention after your changes: Both || and && are at the
same level, though they don't have the same priority/level. Putting an
IF-clause in multiple lines as in thise case has the reason to allow
to see the levels *without* actually examing the brackets.
(Well, at least for people being used to read multi-line IFs with this
pattern, the changed version is misleading.)
- the loop at the end of UnoControlDialogModel::implUpdateGroupStructure
could be written with some ::std::copy / std::insert_iterator
(which would make it easier to understand as "it's a copy operation",
though not necessarily shorter to write)
+ there's stil no code for *reading* the GroupName property of a radio
button in a form
Note: items denoted with a + are what I would consider essential before
integrating the feature, items denoted with a - are on the nice-to-have
list or on the a-matter-of-taste list.
Note (2): For the toolkit part, you might also want to approach (perhaps
by attaching the patch to an issue in IZ, to have a central place for
discussion) Malte Timmermann (mt) and Carsten Driesner (cd) - the former
is the original author of the toolkit with still deep knowledge, the
latter is the current owner of the toolkit. Both might have to add
something ...
Ciao
Frank
--
- Frank Schönheit, Software Engineer [EMAIL PROTECTED] -
- Sun Microsystems http://www.sun.com/staroffice -
- OpenOffice.org Base http://dba.openoffice.org -
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
