Bug 23720 - Permit a node to remove useless expand handles
Permit a node to remove useless expand handles
Status: RESOLVED FIXED
Product: platform
Classification: Unclassified
Component: Nodes
3.x
PC Linux
: P3 (vote)
: 3.x
Assigned To: Petr Hrebejk
issues@platform
http://www.netbeans.org/servlets/Brow...
:
Depends on:
Blocks: 17136 28225
  Show dependency treegraph
 
Reported: 2002-05-17 21:46 UTC by Jesse Glick
Modified: 2008-12-22 19:33 UTC (History)
5 users (show)

See Also:
Issue Type: ENHANCEMENT
:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2002-05-17 21:46:54 UTC
Sometimes a node knows (easily) it has no children
now, but might later; it does not want expand
handles to appear. It should be able to indicate
this, and TreeView should check it. I suggested:

public class Children {
    // ...
    /** If true, check for subnodes right away
when displaying,
     * in the hope of removing a useless expand
handle.
     * The default implementation always returns
false.
     * @return true if subnodes are cheap to check
for,
     *         false if this is a performance hit
     * @since 2.xx
     */
    public boolean isLightweightExpandable() {
        return false;
    }
    // ...
}

Yarda suggested making isLeaf non-final (need
details of proposal though).

Check nbdev thread above.
Comment 1 _ hkrug 2002-06-17 08:30:03 UTC
What is the current state of this issue ? Will the target milestone
(3.4) be met ?

Being disturbed by those many turner icons for LookNodes I started to
formulate a proposal like this:

Make `isLeaf()' being non-final and change the semantics of
`isLeaf()'. The new semantics would be better reflected by a name like
`mayHaveChildren()', although the name of the method probably should
not be changed. The new Javadoc expressing the semantics would be
something like:

Returns <code>true</code>, when it is known before calling {@link
#getChildren()} that the node may not have any children, returns
<code>false</code> when {@link #getChildren()} must be called to be
informed about the childs. {@link #getChildren()} must return [@link
Children.LEAF}, when <code>isLeaf()</code> returns <code>true</code>.
{@link #getChildren()} is free to return any valid value including
<code>false</code> when  <code>isLeaf()</code> returns
<code>false</code>. Super-classes overriding this method should fire
of {@link #PROP_CHILDREN} property change whenever the a change is
made affecting the return value of this method.

I don't know but suppose such a change would not require any changes
to the node-tree views (explorer etc.). But the Looks API should be
modified after this issue has been fixed.
Comment 2 _ hkrug 2002-06-17 08:32:06 UTC
Evidently I wanted to say `mayNotHaveChildren()' instead of
`mayHaveChildren()'.
Comment 3 Jesse Glick 2002-06-17 14:54:08 UTC
We are well after feature freeze, this is not an option for NB 3.4.

There are technical problems with the proposed API change.
getChildren() is final because the children of a Node can never change
during its lifetime; and getChildren cannot return false (wrong type).
Nodes also cannot fire PROP_CHILDREN changes directly; the methods to
do so are not protected, they are package-private, because they are
supposed to be fired only by the children themselves.

Suggest a slightly different change:

1. Make isLeaf() nonfinal. Default impl is still getChildren() ==
LEAF. Document that it must return false when getChildren.getCount() >
0. A node which might have children, but knows it doesn't now, can use
whatever Children it likes, but override isLeaf() to return true.

2. Keep Explorer rendering code's algorithm for determining when to
show expand handles. It shows them when !isLeaf(), at least until the
node is expanded and shown to have children.

3. Introduce a new method:

protected final void Node.fireLeafChange();

and a new property for NodeListener's:

public static final String Node.PROP_LEAF = "leaf";

4. Change Explorer code to listen to PROP_LEAF. If a node began as a
leaf, but changes to a non-leaf, give it expand handles. If it was a
non-leaf and had an expand handle, and changes to a leaf, remove the
handle.
Comment 4 Petr Hrebejk 2002-06-17 16:06:37 UTC
I'm currently on implementing and testing Jarda's proposal to add
new method protected final setChildren( Children ) into the Node.
It seems to work. IsLeaf can stay final. I also added PROP_LEAF
which is fired whenever the Children change from no LEAF to LEAF and
vice versa. Because setChildren is final we probably do not need the
fireIsLeaf method.
I didn't change the explorer yet. As soon as I'll have the tests
finished I'll put it into branch. I would like to put it into 3.4
but I know that it's a feature and API change so I'll probably wait
until the 3.4 will be branched.

Comments are of course welcome.
Comment 5 _ hkrug 2002-06-17 16:17:59 UTC
Hi Petr,

Concerning Jesse's proposal (which was similar to my proposal with my
misunderstandings removed) I could understand how to integrate the
changes into the looks API. Simply adding a method `Look.isLeaf(..)'
or `Look.mayHaveChildren(..)' or `Look.mayNotHaveChildren(..)' to the
`Look' class and overwrite `Node.isLeaf()' in the `LookNode' class.

Is it checked that your modification can be reflected similarly easy
in the `Looks' API ? How ? If not, could you communicate with the
`Looks' developers concerning this issue ?

Good luck with your implementation. I hope that you'll get it into
3.4.
Comment 6 Jesse Glick 2002-06-17 17:37:47 UTC
Petr's solution sounds fine, provided it can work with Looks. It
sounds like it will. Don't forget FilterNode will probably need some
changes to match.

Re. getting it into 3.4 - as release coordinator I don't think this is
a good time. We are after feature freeze and API changes this late are
frowned upon. Especially as the feature affects Nodes, a pretty
central part of NetBeans, and might have performance or stability
implications we don't know about yet. It is not a bugfix, and doesn't
seem to be a critical feature for any module; the only negative effect
on the user of the current situation is having an expand handle show
where none is necessary, which seems pretty cosmetic.
Comment 7 Torbjorn Norbye 2002-06-17 18:48:48 UTC
FWIW, I'm working around this in the tasklist module until
this issue is fixed by recreating the parent node when the
first child is added.  In other words, I create nodes
as leaves (unless I know they have children at the time
I create the node).  Then when I add a subtask, I go and delete
the parent, then recreate it as a non-leaf. Unfortunately I
had to refresh the grandparent node after the parent deletion,
otherwise it wouldn't work, so it has a flicker visual side
effect, but it's better than having turners on every single
task node, since most tasks do not have subtasks.
Comment 8 Jesse Glick 2002-06-17 21:01:02 UTC
Re. refreshing the grandparent node - what do you mean, out of curiosity?

Another problem besides flicker with recreating the parent node is
that you will lose the node selection if it was on the parent.
Comment 9 Torbjorn Norbye 2002-06-17 21:20:37 UTC
I'm adding a new task. If it's parent doesn't have any other
subtasks already, its corresponding node is a leaf. Therefore,
I have to recreate the parent's node. To do that, I have to
first delete it, and for this to be discovered by the nodes,
I have to go to the grandparent (the parent of the node I'm
deleting) and fire a property change on its children object.
I then add the parent in, and now that it has a child it's
not created as a leaf. I then notify the grandparent again such
that the parent node is shown along with its new subnode.
I was hoping to drop the intermedia notification that the
parent has been deleted but when I did that things stopped 
working.

Yes, I've noticed the dropped selection as well, but in my
case it wasn't a problem, because when you create a new node,
I always select it (not just because I want the parent node
expanded, but because it felt good from a UI perspective to
immediately get feedback of what it created.)
Comment 10 Jesse Glick 2002-06-17 21:47:44 UTC
I'm not sure why you need to go through this delete / fire changes in
children sequence... assuming the grandparent is using e.g.
Children.Keys, just calling setKeys should do it in one step.
Comment 11 Petr Hrebejk 2002-06-18 14:37:18 UTC
OK I've checked in new Node.java and Children.java (plus one test
for the new method setChildren( Children ) into openide/nodes.) 

The name of the branch is Nodes_23720. Who ever is interested pleas
look at it and comment.  

The explorer is not changed yet. I'll let you know. Using this issue.
I'll also check whether this solution works with Looks.
Comment 12 _ hkrug 2002-06-18 14:46:28 UTC
I suppose the 3.4 branch will be created soon. Will you then integrate
the change into the main branch ? In this case I will wait the
presumably few days until the branching.

Thanks a lot !
Comment 13 Jesse Glick 2002-06-18 15:28:23 UTC
Holger: current schedule for branching is circa July 3rd. And yes,
assuming the change works, it could go into the trunk as soon as the
branch is made.
Comment 14 Petr Hrebejk 2002-06-18 15:32:14 UTC
Sure I will integrate it into trunk. I'll also try to change the 
explorer and Looks APIs to reflect the changes.
Comment 15 Petr Hrebejk 2002-06-24 09:49:55 UTC
Four more files added into the Nodes_23737 branch:

org.openide.explorer.view.NodeModel.java
org.openide.explorer.view.NodeListModel.java
org.openide.explorer.view.NodeTreeModel.java
org.openide.explorer.view.VisualizerNode.java

Eplorer should now react properly on changes from LEAF to noLEAF and
vice versa.

I'm not sure at all that the changes are OK, so comments welcome
Comment 16 Petr Hrebejk 2002-06-24 11:14:44 UTC
Moving this issue to 4.0. 
Comment 17 Petr Hrebejk 2002-06-24 16:23:22 UTC
Patched version of FilterNode added to the branch. The patch fixes 
the issue #17136. It passes the FilterNodeTest written by Jarda.
However, comments are welcome because, you never know ...
Comment 18 Jaroslav Tulach 2002-06-25 07:37:46 UTC
Reviewed: Explorer and FilterNode changes in branch Nodes_23720 look
reasonable.
Comment 19 Jesse Glick 2002-06-25 13:14:04 UTC
Petr I don't see a base tag on the branch, how are you supposed to
diff the changes?

http://www.netbeans.org/devhome/sources/cvs-branches.html
Comment 20 Petr Hrebejk 2002-07-15 13:03:43 UTC
Branch Nodes_23720 merged into mein trunk. Thus fixed in main trunk.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo