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.
The goal is to open UI panel for Platform and Suite selection, used in first step of NetBeans Module project creation wizard through friend package. vmd.componentssupport is a friend project which will use exposed UI. wizard in vmd.componentssupport creates Java ME Visual Designer component. Which is actually a NetBeans Module project with specific set of descriptor classes and xml updates. While result is NetBeans Module project, UI Review (issue 136248) required vmd.componentssupport to have the same UI for module platform and suite selection as apisupport.project. issue 142283 was filed with request to expose existing implementation through friend package. related wiki page: http://wiki.netbeans.org/PlatformSuiteUIExposedFromApisupport
please find latest patch version in issue 142283
Y01 I'd like to see minimalistic API. A single class with factory method public static WizardDescriptor.Panel create() would be enough, imho. Maybe plus public static setters like: public static void setProjectFolder(wizardDescriptor, folder) and public static FileObject getProjectFolder(wizardDescriptor). Y02 Replace public static final String with methods like set/getProjectFolder Y03 No need for updateUI method. Instead the panel shall listen on changes in WizardDescriptor properties via PCL Y04 As I am suggesting to simplify the API but make the logic more complicated: That means we also need tests to verify that the functionality really works. Please cover the "Usage" scenario with automated test. Btw. I really like the "Usage" section, imho it is the most valuable info about the need for the API change.
jtulach, I can follow your recommendations but have some doubts. >> A single class with factory method public static WizardDescriptor.Panel create() would be enough, imho. It wasn't clear from my description, that plan was to expose only part of wizard panel UI (screenshot is attached). WizardDescriptor.Panel on "Name and location" step in apisupport has too complex Visual Component, to expose it completely. It supports NB module project types that vmd.componentssupport will not handle. Anyway will change to use class with factory method. >> Y02 Replace public static final String with methods like set/getProjectFolder Do you mean to replace read(WizardDescriptor) and store(WizardDescriptor) by getter/setter for 5 properties? Can do, but this will make API less minimalistic. And I will have to provide wizardDescriptor using some non-traditional way (commonly panel gets wizardDescriptor on read/store/validate). >>Y03 No need for updateUI method. Instead the panel shall listen on changes in WizardDescriptor properties via PCL agree. Have added add/removePCL and updateUI methods just to make API more clear.
Created attachment 67946 [details] inner panel that will be exposed
I change API in the following way: - rename updateUI to setProjectFolder(WizardDescriptor) - remove store/read methods updated property values will be stored to WizardDescriptor right after modification - remove add/removePropertyChangeListener methods outer panel will listen to changes in WizardDescriptor - remove validate method updated property values will be validated and error message will be stored to WizardDescriptor right after modification 5 property names are leaved as "public status final String" attrs: ACTIVE_PLATFORM IS_STANDALONE IS_SUITE_COMPONENT SUITE_ROOT IS_NETBEANS_ORG
OK, I am looking forward to see new patch.
New patch is in the following attachments. wiki page is updated: http://wiki.netbeans.org/PlatformSuiteUIExposedFromApisupport
Created attachment 68291 [details] patch for apisupport.project
Created attachment 68292 [details] patch for vmd.componentssupport to use API exposed by previous patch
Created attachment 68379 [details] updated patch for apisupport.project. Contains test and some updates in implementation.
I don't really have time to review in detail. I hope rmichalsky is doing so? [JG01] Why does template_netbeansorg exist? I don't think there is any need for this template to be available inside the nb.org source tree. [JG02] It might be possible to replace TestPCL with MockPropertyChangeListener (o.o.util.test).
Thanks for review >> I hope rmichalsky is doing so? >> [JG01] Why does template_netbeansorg exist? I don't think there is any need for this template to be available inside the nb.org source tree. It is necessary to support absolutely the same cases as Nb Module project creation wizard does (as was requested in UI review). >>[JG02] It might be possible to replace TestPCL with MockPropertyChangeListener (o.o.util.test). thanks. will do.
>> [JG]I hope rmichalsky is doing so? He has reviewed before jtulach.
Created attachment 68511 [details] patch for apisupport.project with test updated according to jglick comments
Y11 Why there is not void setProjectFolder(WizardDescriptor settings, FileObject folder)? Why the method has so complicated Javadoc? Additional P3 comments: Y12 I still do not like (Boolean)wizardDescriptor.getProperty(ModuleTypePanel.IS_NETBEANS_ORG) and similar I'd like it to be replaced with ModuleTypePanelCreator.isNetBeansOrg(wizardDescriptor). Y13 I would get rid of the ModuleTypePanel interface completely and instead had: class ModuleTypePanelCreator { public static boolean validate(WizardDescriptor wd); public static void setProjectFolder(WizardDescriptor wd, FileObject folder); public static JComponent createComponent(WizardDescriptor wd); } then the client API would work as follows. Prepare wd, createComponent(...) display it somewhere, use static methods to change values in wd and get values from wd. the implementation would display a component which adds listener to wd and observe all changes in the wd values and update its state according to them and vice versa.
>>Y11 Why there is not void setProjectFolder(WizardDescriptor settings, FileObject folder)? Why the method has so complicated Javadoc? ok. >> Y12 I still do not like (Boolean)wizardDescriptor.getProperty(ModuleTypePanel.IS_NETBEANS_ORG) and similar I'd like it to be replaced with ModuleTypePanelCreator.isNetBeansOrg(wizardDescriptor). ok. But I have to leave three public static string attributes to filter property change events. >>Y13 I would get rid of the ModuleTypePanel interface completely and instead had: ok Thank you.
P3s took much more time than P1. the following patches include you comments. - setProjectFolder is updated - all wizardDescriptor.getProperty are replaced with getters with (WizardDescriptor) parameter - all communication with panel is performed through ModuleTypePanel static methods static JComponent createComponent(WizardDescriptor) static boolean validate(WizardDescriptor) static void setProjectFolder(WizardDescriptor) and PCL for wizard descriptor. BUT: I had to leave "public static final String" attributes to filter PCL events.
Created attachment 68575 [details] patch for apisupport.project with jtulach commnets
Created attachment 68576 [details] patch for vmd.componentssupport with jtulach commnets
I can imagine that those P3 took more time. That is why I made them P3s. But I like the result. I think we are getting more condensed API. Y21 Missing private constructor of ModuleTypePanel - you do not want people to make instances of that class, I think. Easy fix, I guess. Y22 Same string constants are still public. For example IS_STANDALONE_OR_SUITE_COMPONENT is public, while API users shall use the isSuiteComponent(...) method. Please hide those constants from API. A bit of work, I guess. Y23 A new, not mentioned issue: Versioning. Increase the version of apisupport.project module to X.Y, add @since X.Y to the newly added class, make a note in apichanges.xml, if there are some. Simple, I guess. Good luck and thanks for having patience with my comments and fulfilling them so greatly.
>> Y21 Missing private constructor of ModuleTypePanel - you do not want people to make instances of that class, I think. done >>Y22 Same string constants are still public. For example IS_STANDALONE_OR_SUITE_COMPONENT is public, while API users shall use the isSuiteComponent(...) method. Please hide those constants from API. they were used to filter PropertyChangeEvents. new method static boolean isPanelUpdated(PropertyChangeEvent) is added for this purpose. >>Y23 A new, not mentioned issue: Versioning. Increase the version of apisupport.project module to X.Y, add @since X.Y to the newly added class, make a note in apichanges.xml, if there are some. done Thanks for comments.
Created attachment 68611 [details] patch for apisupport.project with jtulach commnets from Thu Aug 28 21:52:03
Created attachment 68612 [details] patch for vmd.componentssupport with jtulach commnets from Thu Aug 28 21:52:03
almost two weeks are passed from issue was filed. All comments are applied. I mark issue as fixed.