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.
| Summary: | Bring forward / Send Backward | ||
|---|---|---|---|
| Product: | platform | Reporter: | _ oracmpalmer <oracmpalmer> |
| Component: | Graph | Assignee: | issues@platform <issues> |
| Status: | RESOLVED WONTFIX | ||
| Severity: | blocker | CC: | apireviews, jglick |
| Priority: | P2 | Keywords: | API_REVIEW_FAST |
| Version: | 6.x | ||
| Hardware: | Macintosh | ||
| OS: | All | ||
| Issue Type: | ENHANCEMENT | Exception Reporter: | |
| Attachments: |
Patch to Widget
WidgetSupport patch Changes included Final Patch Proposal Mercurial final patch Mercurial final patch with new files |
||
|
Description
_ oracmpalmer
2007-11-08 23:36:48 UTC
Created attachment 52764 [details]
Patch to Widget
Created attachment 52765 [details]
WidgetSupport patch
Idea is good and we may add it to the CVS after NetBeans 6.0 release. Comments: 1) Widget.orderChildren method is working with parentWidget.children but it makes more sense to modify the Widget children directly instead. 2) WidgetSupport class has to be final. 3) Integer.intValue is not necessary because of auto-boxing in Java. 4) Maybe we can use arrays instead of Lists - it could be faster i.e. Widget.reorderChildren (int... permutation) the int[] could be used in WidgetSupport class too 5) Missing @param description in Javadoc in WidgetSupport P.S.: You can use one patch file for all changed/new/removed files. >>1) Widget.orderChildren method is working with parentWidget.children but it makes more sense to modify the Widget >>children directly instead. I was following the lead in the methods that exist in Widget already. That is what they do? Perhaps it should be on "Scene" >>2) WidgetSupport class has to be final. Ok >>3) Integer.intValue is not necessary because of auto-boxing in Java. OK >> 4) Maybe we can use arrays instead of Lists - it could be faster i.e. >>Widget.reorderChildren (int... permutation) >>the int[] could be used in WidgetSupport class too Sure if you really believe it will help. >>5) Missing @param description in Javadoc in WidgetSupport OK Re: Re: 1) I assume you mean Widget.bringToFront() - that widget does not take any parameter therefore it is itself that has to be send to particular z-order. In Widget.orderChildren(...) - you are saying that the Widget children should be changed. Re: Re: 4) Ok. I am not 100% sure - I just think it would be faster and enough convenient because developers would have pre-create the array of particular size anyway. Therefore there is not explicit need for using List. Created attachment 54733 [details]
Changes included
I have made the suggested changes. I still don't think its too important about the List<Integer>.
>>Re: Re: 1)
>>I assume you mean Widget.bringToFront() - that widget does not take any parameter therefore it is itself that has to be
>>send to particular z-order.
>>In Widget.orderChildren(...) - you are saying that the Widget children should be changed.
I updated the api to include the specific parent is that sufficient? I still don't have commit access so you will have to commit the changes.
Created attachment 54756 [details]
Final Patch Proposal
I have updated the code a little bit: Mainly Widget.orderChildren method changes children of THE widget and WidgetSupport class methods does not require parentWidget parameter. I have attached the patch in "Final Patch Proposal". Chris, could you look at it? Let me know, if you agree with it. If so, I will post it to API review. Thanks. Well I tested the patch with our application and it appears to work the same. I think we can commit and test. Once api review accepts. The change is adding a new Widget.orderChildren method and a new WidgetSupport class. The patch contains documentation, javadoc, automated test (test.apichanges.WidgetOrderTest), ... Asking for API review. Y01 I've heard somewhere, that assert shall not be used to verify API method parameters. Y02 Write a test to verify behaviour on incorrect parameter to "orderChildren" method. Y03 As far as my own experience (Nodes API) says, reordering with permutation is one of the biggest mistake one can do. First of all one needs to compute the permutation, then apply it. Who guarantees data structures are not modified meanwhile? Better to have reorder(Widget[] preferredOrder) that does all of this itself, imho. Re. Y01: you can use org.openide.util.Parameters for checking method parameters. Created attachment 56731 [details]
Mercurial final patch
I have attached a new patch file with Y01, Y02, Y03 implemented. Y02 I cannot find any test in the "final" diff Y04 Is the method supposed to be threadsafe or what is its supposed threading usage? I am asking because two threads accessing it can cause big mess. Created attachment 56734 [details]
Mercurial final patch with new files
Y03: Sorry, I have forgotten to include new files into the patch. Y04: The library is meant to be used in AWT-thread only - mentioned in documentation. It would be good to check add checking for AWT-thread to all API methods. Still active? Probably not active. |