Community
Participate
Working Groups
Created attachment 120605 [details] ValidationStatusProvider that tracks changes to the targets of a contexts bindings Hi, I would like to introduce the following api methods on WizardPageSupport: public void setMessage(String message, int type) public void setErrorMessage(String message) Manual calls to IWizardPage.setErrorMessage(...) or setMessage(...) do not go well together with WizardPageSupport. I.e. you want the messages to be removed as soon as the user makes a change in the ui. The concrete use case is some costly validation (requiring network access). We only want to do that when the users tries to proceed to the next page, i.e. inside of IWizardPage.getNextPage(). The attached implementation of ValidationStatusProvider can be used like: <code> manualValidation = new ManualValidation(context); context.addValidationStatusProvider(manualValidation) ... ... manualValidation.setStatus(ValidationStatus.error("some error message")) </code> However I would prefer this to be integrated inside WizardPageSupport. If there is a chance to get this into 3.5 I would be happy to prepare a patch for the head version of WizardPageSupport. Kind regards, Jan
I don't quite follow you. Could you provide a snippet that shows what you're trying to accomplish?
Hi, sorry for being unclear. Imagine a wizard with a text field for a url. This is bound to a model element of type URL. So we have to validate and convert the string from Text.getText() to URL. There is a cheap validation (no MalformedURLException) that can be done on each key stroke. No problem until here. There is also an expensive validation (actually reading from that url and validating the content) that we want to do only if the user finished typing (pressed the next button) class MyWizardPage extends WizardPage { ... @Override public IWizardPage getNextPage() { //get something from remote and validate it if (!isValid(load(url))){ setErrorMessage("some error message"); return this; } } ... } Now if the user changes some input, e.g. the url, this error message is potentially invalid. Hence the it should disappear. Is that anyhow clearer now? Kind regards, Jan
This sounds like something that should be done using a ValidationStatusProvider, although the need to make it appear only when the user clicks Next is a tough one. If you could add a button "Validate URL" instead of validating when the user clicks Next, then use a WritableValue<Boolean> to indicate whether the Validate URL button has been clicked. At that point you could use a MultiValidator to compute the long-running validation, but only when the WritableValue is true. (When false you could return a ValidationStatus.cancel() status indicating that the user needs to click "Validate URL" to continue. final IObservableValue validateClicked = new WritableValue(realm, Boolean.FALSE, Boolean.TYPE); MultiValidator urlValidator = new MultiValidator(realm) { protected IStatus validate() { Boolean validatedWasClicked = (Boolean) validateClicked.getValue(); if (validatedWasClicked.booleanValue()) { // perform URL validation, preferably using // getContainer().run(boolean, boolean, IRunnableWithProgress) } else { return ValidationStatus.cancel("Click Validate URL to continue"); } } }; // Don't forget this part dbc.addValidationStatusProvider(urlValidator);
In the above example, when performing URL validation be sure to access the URL through the model-side observable so that further changes to the observable will trigger revalidation. Also, if the URL validation fails be sure to reset validatedClicked to false.
Mmh, now I was clear on one part, the other got lost. Thanks for your explanations but for that kind of question I would rather have asked on the newsgroup. Actually I think I already have the solution: The attached java class is indeed a subclass of ValidationStatusProvider. What it adds is a setStatus method. When called a change listener is attached to each target of each validation status provider contained in the DataBindingContext. The listener resets the status (wrapped in a WritableValue) to Status.OK_STATUS and deregisters the listener (WizardPageSupport does something similar to not show an error the first time the wizard is opened) This way one can set a status (for example from inside getNextPage) that will be shown only until the user does a change to the ui (one of the targets). I am not sure this is "the solution" for the problem but I did not come to a better one. Why I opened this enhancement request? I would like to have the provided (or any other) solution integrated with WizardPageSupport and thus delivered with JFace Databinding. To do this I would propose to reflect the two methods from WizardPage (actually setErrorMessage and setMessage are in DialogPage I think) in WizardPage support. Maybe having a setStatus method would actually be better? I am willing to prepare a patch for review if there is a chance to get this included, i.e. you at least acknowledge my use case is valid and the solution is at least on the right track.
Ok, I did not notice your attachment when I left my previous comments. I think I understand your problem better now. I wouldn't be opposed to adding WizardPageSupport.setStatus() but I'd like to run it by Boris to get his input first. There may be another, better option we haven't considered.
Could you add a ValidationStatusProvider to the data binding context? One that wraps a WritableValue containing the IStatus that represents the extra validation result?
That't what we currently do. (see attachment) I just think it should be integrated into the framework.
There are three distinct behaviors in the patch, namely: 1) Implement a concrete validation status provider with a mutable validation status observable. Potentially we could just do this right in ValidationStatusProvider and allow for subclasses to override. 2) Provide an easy way to be notified of changes on any target observable of any binding in a DataBindingContext (this could be extracted from DialogPageSupport). 3) Reset the validation status to some value when a change is observed on one of the above mentioned target observables. I think it's reasonable to implement (1) and (2) as individual APIs, but (3) is a special case that is tied to the design of your application. I'm not sure this particular behavior would be useful (or even desirable) for a wider audience.
I agree with Matt. The problem with adding support to WizardPageSupport instead of using ValidationStatusProvider is that this functionality would not be available for other contexts (e.g. DialogPageSupport, TitleAreaDialogSupport...)
Created attachment 123299 [details] WritableValidationStatusProvider This should satisfy case (1) in comment #9
Created attachment 123427 [details] TargetsListener This should satisfy case (2)
Created attachment 123428 [details] mylyn/context/zip
Created attachment 123429 [details] DialogPageSupport This adapts DialogPageSupport to the newly introduced TargetsListener
Created attachment 123430 [details] mylyn/context/zip
I'm considering another approach which depends on bug 263714 (add the ability to observe lists of lists, similar to MultiList). With this you could take a DataBindingContext with the following validation status providers: validationStatusProviders[0].targets == [observable1, observable2, observable3] validationStatusProviders[1].targets == [observable4, observable5] And do the following to get a list of all targets: IObservableList targets = BindingProperties.validationStatusProviders().lists( BindingProperties.targets()).observe(bindingContext); where targets == [observable1, observable2, observable3, observable4, observable5] This would trivialize the implementation of TargetsListener
Jan, could you combine your prior patches and and post them as a single patch? I need to evaluate them all together (and check whether the LOC is below the threshold requiring an IP review), and the combined patch needs to be posted by you for IP logging purposes.
Created attachment 127067 [details] TargetsListener and DialogPageSupport Sure, here you go.
Created attachment 127068 [details] patch depending on yours for 263714 This should do as soon as work for 263714 is finished.
Ran out of time for 3.5
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.
