Bug 58457 - Project Customizer should allow disabling of OK buttton on invalid data
Project Customizer should allow disabling of OK buttton on invalid data
Status: CLOSED FIXED
Product: projects
Classification: Unclassified
Component: Generic Projects UI
4.x
All All
: P2 (vote)
: 5.x
Assigned To: Martin Krauskopf
issues@projects
: API, UI
Depends on:
Blocks: 61032
  Show dependency treegraph
 
Reported: 2005-04-29 22:18 UTC by Praveen Savur
Modified: 2006-03-24 12:48 UTC (History)
3 users (show)

See Also:
Issue Type: ENHANCEMENT
:


Attachments
patch1 (11.10 KB, text/plain)
2005-07-17 21:02 UTC, Martin Krauskopf
Details
patch2 (marks invalid category) (13.43 KB, text/plain)
2005-07-18 08:22 UTC, Martin Krauskopf
Details
patch3 (permits setting an errorMessage) (21.21 KB, text/plain)
2005-07-18 15:33 UTC, Martin Krauskopf
Details
removing non-API methods from PC.Category (22.81 KB, text/plain)
2005-07-21 20:02 UTC, Martin Krauskopf
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Praveen Savur 2005-04-29 22:18:16 UTC
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.
Comment 1 Jesse Glick 2005-04-29 22:33:47 UTC
Sounds reasonable.
Comment 2 Petr Hrebejk 2005-04-30 14:20:33 UTC
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.
Comment 3 Martin Krauskopf 2005-07-15 14:30:33 UTC
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.
Comment 4 Martin Krauskopf 2005-07-17 18:46:21 UTC
I'm will be working on this. I will attach a patch later.
Comment 5 Martin Krauskopf 2005-07-17 21:01:47 UTC
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).
Comment 6 Martin Krauskopf 2005-07-17 21:02:24 UTC
Created attachment 23137 [details]
patch1
Comment 7 Martin Krauskopf 2005-07-18 08:22:49 UTC
Created attachment 23139 [details]
patch2 (marks invalid category)
Comment 8 Martin Krauskopf 2005-07-18 15:33:31 UTC
Created attachment 23142 [details]
patch3 (permits setting an errorMessage)
Comment 9 Martin Krauskopf 2005-07-18 15:34:03 UTC
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).
Comment 10 Jaroslav Tulach 2005-07-21 10:37:25 UTC
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. 
 
Comment 11 Martin Krauskopf 2005-07-21 13:51:19 UTC
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.
Comment 12 Martin Krauskopf 2005-07-21 17:00:09 UTC
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 ;)
Comment 13 Martin Krauskopf 2005-07-21 20:02:20 UTC
Created attachment 23225 [details]
removing non-API methods from PC.Category
Comment 14 Martin Krauskopf 2005-07-21 20:08:40 UTC
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...
Comment 15 Martin Krauskopf 2005-07-21 20:11:54 UTC
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.
Comment 16 Jaroslav Tulach 2005-07-22 10:16:12 UTC
This test will fail. I am not sure if that is good thing. 
 
category.setErrorMessage(null); 
assertNull(category.getErrorMessage()); 
 
Comment 17 Martin Krauskopf 2005-07-22 10:31:54 UTC
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.
Comment 18 Martin Krauskopf 2005-07-28 10:01:43 UTC
Thanks for review. I'll commit this week with an enhanced javadoc noting about
setErrorMessage(null) case.
Comment 19 Martin Krauskopf 2005-07-28 15:28:18 UTC
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
Comment 20 Martin Krauskopf 2005-07-28 15:31:10 UTC
Forgotten

Checking in apichanges.xml; 1.15 --> 1.16
Checking in nbproject/project.properties; 1.10 --> 1.11
Comment 21 Tomas Danek 2005-12-15 12:04:52 UTC
verified.


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