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.
In org.netbeans.spi.project.ui.support.ProjectCustomizer.createCustomizerDialog(.....) has to expose some API to disable the 'OK' button. Use Case: For my project customizer I want to disable the 'OK' button if the user has entered invalid values in some of the fields. And enable the 'OK' button only if the user has entered valid values. But currently there is no API to disable the OK button.
Sounds reasonable.
Hmm, does it. What if you put some nocense into one tab and then switch to other one the OK remains disabed and the user has very litte chance to find where the initial bug was. But I admit that it is not an api issue.
Re: "What if you put some nonsense into one tab...." Just show that tab in "nb.errorForeground" color - than user can find immediatelly what categories are incorrect. Also it would be nice to have a chance to set an errormessage - like in Wizard (I will fill up another enhancement). Anyway we will need to fix this for the new apisupport. So do you have any plans to fix this for 4.2 Hrebejku? If not, I will have to do so. Probably easy answer for you ;) Reassign back if you are already working on this or planning/want to work on it. I'll begin to work on the pach otherwise. Jano if you have any objections/opinions regarding UI, please add a comment.
I'm will be working on this. I will attach a patch later.
The following patch adds an ability to control validity state of the customizer dialog - i.e. whether OK button should be enabled. It simply adds method ProjectCustomizer.Category.setValid and ProjectCustomizer.Category.isValid. OK button is enabled *only* if all customizer's categories are valid. Patch doesn't contain selection of invalid categories yet - but this is implementation detail :). Thanks for the review (of the first draft).
Created attachment 23137 [details] patch1
Created attachment 23139 [details] patch2 (marks invalid category)
Created attachment 23142 [details] patch3 (permits setting an errorMessage)
I added patch3 which permits clients to set an error message for individual categories. Although this is related to the issue 61036 I believe that it makes sense to integrate those two issue with one patch, since they are closely related to each other (however still usable separately). The patch3 only adds ProjectCustomizer.Category.setErrorMessage and PC.C.getErrorMessage which are automatically shown in the bottom of a project customizer (similar to wizards). I already have a successful implementation within an API support module (locally, I could provide diff).
My 3 worries: I feel that there will be a need to add behaviour of "needs_validation". E.g. let the ok button be enabled and when pressed invoke a callback that allows to do validation. This is needed in case when computing of the valid state requires complex operations, connections to network, etc. This does not need to be fixed now, but I want the submitter to think about this and be sure that such kind of support can be smoothly added in future. With the introduction of valid, errormessages, ok button state, the behaviour of the UI gets more complicated then something that could be simply verified by a human. Now the customizer framework is complex enough to deserve tests. Otherwise we are likley to face regressions after next integrations. Especially those tests tracking the desired interactions between category state, node display name, state of ok button, ability to switch to different category when one is not valid or contains error message deserve automatic verification, as no qa person can do this by just clicking UI. Last worry is about the size of the API change. As far as I can tell it contains methods that are not needed to achieve any usecase and just complicate the API for no reason. Of course I can be wrong, but in such case, please show me the use case. I am talking about methods like: Category.isValid, Category.getErrorMessage and especially add/remove and fire listeners. Those shall not be public as no client is supposed to use them (imho) and they just complicate the test scenarious that need to be written. Make them package private for now and save some work on tests.
My 3 worries: > re worry #1 (needs_validation): I don't see any reason why it wouldn't be possible to implement such a feature in the future. I think that a "needs_validation" feature concerns entire Project Customizer whereas my API change concerns only inidividual Categories. So they are presuambly unrelated since a client firstly could check user's input and eventually disable/enable OK button with "categories" API and then possibly also validate a whole customizer with another API (needs_validation_API) which should be independent on this one. i.e. if the client wants to check whole customizer she/he doesn't need to touch API I propose (it would behave like now, without the "categories" API). > re would #2 (it's getting complex - where are tests?) I think that changes are really simple. So no reason to worry too much :) But of course I agree that tests are needed. Anyway I would like to integrate ASAP since I would like to start using it in the APISupport module. And because supposed API is clear enough (I hope) I would file a P2 and write them as soon as I have a little time for it :). Maybe this weekend? > re would #3 (get rid of superfluous API) Nice catch, thanks. These method are actually called only within the projectuiapi module and are not needed outside. I'll get rid of them. isValid and getMessage can be added later if some reasonable usecase appears.
I thought about possible usecases and I think that we should keep isValid and getErrorMessage in ProjectCustomizer.Category. If we remove them it would force clients to keep a state of individual categories in separate storage which is IMO bad. Note that it doesn't ease me a work too much. I still have to move other three methods out, so I don't want to keep them just to help my self ;)
Created attachment 23225 [details] removing non-API methods from PC.Category
The patch4 removes non-api methods from PC.Category. So now it is clear from the API point of view and I would like to commit the patch on Monday. Also this is "fastest" implementation using static Map in Utilities (don't know about better class now). We could use Lookup mechanism or something more satisfactory. But this is implemenation detail. I'm also planning to read http://openide.netbeans.org/tutorial/api-design.html advertised by Yarda :) so the final implementation can change...
Also CategoryWrapper doesn't make to much sense (I forgot to rename it to something more sensible), but that doesn't matter too much since it is not public API.
This test will fail. I am not sure if that is good thing. category.setErrorMessage(null); assertNull(category.getErrorMessage());
I think it's ok. It is convenience like with a JTextComponent. JTextComponent tc = new JTextField(); tc.setText(null); assertNull(tc.getText()); But definitely should be documented.
Thanks for review. I'll commit this week with an enhanced javadoc noting about setErrorMessage(null) case.
Fixed Checking in modules/project/uiapi/CategoryChangeSupport.java; new 1.1 Checking in modules/project/uiapi/CategoryView.java; 1.1 --> 1.2 Checking in modules/project/uiapi/CustomizerDialog.java; 1.2 --> 1.3 Checking in modules/project/uiapi/CustomizerPane.java; 1.2 --> 1.3 Checking in modules/project/uiapi/Utilities.java; 1.5 --> 1.6 Checking in spi/project/ui/support/ProjectCustomizer.java; 1.2 --> 1.3
Forgotten Checking in apichanges.xml; 1.15 --> 1.16 Checking in nbproject/project.properties; 1.10 --> 1.11
verified.