Hi Noel,
thanks for your recent changes, this is going to look better and better ...
Two non-negligible (IMO) issues:
+ removing an embedded image from a control is possible in some
round-about way only: You need to set a linked image, and then remove
it. This is a pretty bad user experience.
I think this could best be solved together with a placeholder: If we
would display a placeholder such as "<embedded graphics>", then for
the user it would be obvious (hopefully) that pressing "Del" in this
case will delete the embedded graphics from the control.
However, this requires adding placeholder functionality to the
SvtURLBox, since of course we want it to handle the placeholder text
as one big chunk only (i.e. when the user starts to type, the place
holder is completely removed). Not sure how difficult this would be.
+ double-clicking an image control does not allow for non-linked images.
Missed this the last time - the implementation for this is in
ImageControl.cxx, method implInsertGraphics.
Care needs to be taken here: if the method "hasField()" returns true,
then the control is bound to a database column, in which case the
"Link" checkbox should be disabled. If it is "false", it should
be enabled (and checked by default?).
Minor issues (which I mention only since this is an anankasm of mine
when doing code reviews :):
- sVal in FormComponentPropertyHandler::setPropertyValue is superfluous
- string variables usually start with "s" (ObjectID in
FormComponentPropertyHandler::setPropertyValue)
- do you know "makeAny( <value > )"? Saves the
Any aValue( <value > );
useValue( aValue );
construct you often use.
- The "// Bit of a hack to ..." comment in
FormComponentPropertyHandler::impl_browseForImage_nothrow is outdated.
- isImageResourceURL and isGraphicObjectURL could be merged into a
single "isSupportedURL" or so, lowering future extension costs this
way
Then, after stumbling about GraphicObjectFactory again ... what about
naming it "GraphicObject" only? It would be a) better conform to the
habit of naming the service which implements "XFoo" just "Foo" b) spare
a few letters to type :)
Of course, the constructors should be "create" and "createWithId" then.
Finally - cool feature, really. Adding the same image to multiple
controls indeed adds it only once to the file, and such ... I like it.
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]
