This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 37626 - Fast track review of PropertyPanel rewrite API changes
Summary: Fast track review of PropertyPanel rewrite API changes
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Explorer (show other bugs)
Version: 3.x
Hardware: All All
: P1 blocker (vote)
Assignee: apireviews
URL:
Keywords: API_REVIEW_FAST
Depends on: 37779 37780
Blocks: 31896
  Show dependency tree
 
Reported: 2003-12-01 10:21 UTC by _ tboudreau
Modified: 2008-12-22 15:52 UTC (History)
0 users

See Also:
Issue Type: TASK
Exception Reporter:


Attachments
JDiff diff of PropertyPanel class (note some mentioned methods are simply overrides of JComponent methods) (14.93 KB, text/html)
2003-12-01 10:22 UTC, _ tboudreau
Details
Javadoc for PropertyPanel from the branch (110.94 KB, text/html)
2003-12-01 10:24 UTC, _ tboudreau
Details
Diff of explorer docs, arch answers & openide-spec-version (9.32 KB, patch)
2003-12-01 11:17 UTC, _ tboudreau
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description _ tboudreau 2003-12-01 10:21:33 UTC
As part of issue 31896 (see its dependency tree
for dependencies), I am making some fairly small
changes to the API in
org.openide.explorer.propertysheet.  

On a general level, the purpose of the issue
(along with fixing numerous long-standing bugs) is
to get PropertyPanel using the new, lightweight
rendering/editing infrastructure of the new
property sheet.

As part of this change, use of PropertyModel to
drive a PropertyPanel is discouraged (but it is
not yet deprecated - there is a potentially
legitimate use case in needing to listen for
changes on a property and reflect them in the UI,
although UIs that change due to non-user generated
events are usually *bad* UI, so the use case is
dubious).  Instead, it is preferred to use
Node.Property objects directly to drive a
PropertyPanel - there was already one constructor
that accepted such an argument.

The following changes are made on the branch:

STABILITY CONTRACT: OFFICIAL

- Deprecate ExPropertyModel - ExPropertyModel does
nothing that cannot be done with a BeanNode or
PropertySupport.Reflection

- Deprecate DefaultPropertyModel 
Reason: it nearly duplicates
PropertySupport.Reflection, and a BeanNode and
PropertySupport.Reflection.  It is better for the
APIs for there to be one well-defined way to do
something.

- Add two constructors:
public PropertyPanel (Node.Property property)
public PropertyPanel (Node.Property property, int
preferences)
Reason:  If we want people to use Node.Propertys
instead, we need to give them a way to do it.

- Add methods:
public Node.Property getProperty() - returns the
property object driving the PropertyPanel.  In the
case where the older PropertyModel API was used,
this may be a wrapper for the PropertyModel.

public void setProperty(Node.Property property) -
sets the property the instance will display.  A
replacement for setPropertyModel - if we encourage
people to use Propertys, we need to give them a
way to get/set them.

Deprecated members:
getPropertyEditor() - deprecated, AFAIK the only
use case for this even being in the API was in the
old property sheet, which embedded property panels

PROP_PROPERTY_EDITOR - property name for the
property editor property.


Added fields:
PREF_TABLEUI - there was already a weak contract
for use of PropertyPanel as a table/tree cell
editor (c.f. TreeTableView), via
setClientProperty("flat",Boolean.TRUE).  Since
this is a common and reasonable usage of a
PropertyPanel, I am promoting this to a stable API
and adding a preferences field to the preferences
bitmask for specifying it.  The old
client-property API still works as well.

STABILITY CONTRACT: FRIEND
A few new client properties are supported for
PropertyPanel.  If the review panel does not feel
they are warranted at this time they may remain
undocumented:

"suppressCustomEditor" - Boolean - a property
panel with this value set to true will not display
a custom editor button, even if a custom editor is
available.  Covered by unit tests on the branch.

"useLabels" - Boolean - a property panel with this
value set to true will use the property display
name as the label for those properties where it is
possible/appropriate.  In particular this is aimed
at the case of a Checkbox boolean editor - a
checkbox indicates that it has focus using its
label, so the most natural UI in a panel is to
have the checkbox behave normally.  By default it
would have no label.  A secondary usage is
described below:

"radioButtonThreshold" - Integer - one of the
problems observed with the now default Projects
project was many cases of combo boxes with only 1
or 2 items.  This UI was because there *could* be
many such items, but commonly would not be.  With
this value set > 0, a PropertyPanel that would
display a combo box will use radio buttons if the
number of options is below the threshold specified
 by this client property.  The default is 0, so
there is no change in default behavior.  Covered
by unit tests on the branch.  While the projects
branch is defunct, there is no reason to believe
this particular UI problem will cease to appear in
NetBeans.

I will attach related documentation to this issue.
Comment 1 _ tboudreau 2003-12-01 10:22:49 UTC
Created attachment 12366 [details]
JDiff diff of PropertyPanel class (note some mentioned methods are simply overrides of JComponent methods)
Comment 2 _ tboudreau 2003-12-01 10:24:02 UTC
Created attachment 12367 [details]
Javadoc for PropertyPanel from the branch
Comment 3 _ tboudreau 2003-12-01 10:27:05 UTC
One unrelated change which occurs on this branch:

Remove method
org.openide.explorer.propertysheet.InplaceEditor.handleInitialInputEvent

I found it is equally effective to just dispatch the appropriate event
to an inplace editor using normal methods.  If an inplace editor needs
to handle it in a different way because it is the initial event, it
knows if it was just added to a container.

The interface InplaceEditor was new with the property sheet rewrite,
and has never been published in a stable release of NetBeans - so not
only is this a compatible change, it is not even a change to the
official APIs.
Comment 4 _ tboudreau 2003-12-01 11:17:34 UTC
Created attachment 12369 [details]
Diff of explorer docs, arch answers & openide-spec-version
Comment 5 Tomas Pavek 2003-12-03 14:29:27 UTC
Adding API_REVIEW_FAST keyword. If nobody objects, this is accepted on
Dec 8th.
Comment 6 Jaroslav Tulach 2003-12-05 10:51:44 UTC
I am amazed. Only small API changes, tons of tests testing various
interactions and even! that correct icon is rendered at desired
possition. I think I could not write that better (not counting that I
have not write UI stuff for years), but still as a reviewer I have to
have some comments:

- next time, please diff -u when generating diffs

- request for better documentation described in issue 37780

- request for providing compatibility bridge put into issue 37779

- my advice is to condider putting all resources (icons, etc.)
directly into org.openide.explorer.propertysheet package, this would
be step towards openide.jar separation

- my advice is to eliminate call to
org.netbeans.core.NonGui.registerEditors() from tests if possible, as
this methods is a candidate for removal. 

- small advice is to create a special category of tests in
openide/test/cfg-unit so people could just start those colorful, joy
to watch tests of propertysheet and do not need to wait for those
boring GUI-less tests of CompilationEngine, etc. (just make sure it is
still executed by default when doing ant -f openide/test/build.xml).

Beyond my two requests above I have no idea what, from architecture
point of view could prevent integration of such beautiful improvement.
Comment 7 _ tboudreau 2003-12-05 12:54:26 UTC
Re org.netbeans.core.NonGui.registerEditors() - this is mainly because
we do test the boolean editors, and the JDK boolean editor doesn't
support getTags().  There are workarounds in the property sheet
package for platform use, where that may be missing, but the tests
rely on our boolean editor.

Would it not be better to separate the core property editors for JDK
classes into their own module?
Comment 8 _ tboudreau 2003-12-09 09:32:28 UTC
Property panel branch integrated into trunk.