jwaliszko / ExpressiveAnnotations

Annotation-based conditional validation library.
MIT License
351 stars 123 forks source link

Enable client-side required indicator #139

Closed michielvanerp closed 7 years ago

michielvanerp commented 7 years ago

I might've missed something, but don't know how to indicate required fields, client-side. For instance, show an asterisk on a label for a text input that is required. I got it working only for 'static' required fields, which have the attribute data-val-required. When the ExpressiveAnnotations attribute 'data-val-requiredif' has been set, I'd like to show or hide the asterisk on the corresponding label, depending on the condition.

This is what I have for fields that are always required:

 $('input[type=text], select').each(function () {
                if (undefined != $(this).attr('data-val-required');) {
                    var label = $('label[for="' + $(this).attr('id') + '"]');
                    var text = label.text();
                    var asterisk = '<span class="asterisk">* </span>';
                    if (text.length > 0) {
                        label.prepend(asterisk);
                    }
                }
            });
jwaliszko commented 7 years ago

You need to trigger the element validation first, in order to know the expression result. Based on the result you know whether the requirement condition is satisfied, so you know whether its corresponding asterisk should be still shown or not.

michielvanerp commented 7 years ago

Thanks for your quick reply, I now see my question wasn't completely accurate: I want to always show the asterisk when a field is required, based on a condition. Even if a valid value has been entered, I want to keep the indicator. I looked into triggering eavalid but this seems to only provide the type, whether the condition has been met (valid) and the expression. Is there an easy way to find out whether the field is required? Or do I need to resolve the expression myself to find out whether the field is required?

michielvanerp commented 7 years ago

Valid doesn't seem to indicate whether e.currentTarget.name is required, valid seems to indicate that the condition was met. :) On my viewmodel, I have this attribute on property Frequency: [RequiredIf("Status > 1")] If I console.log your code with Status = 2 - so that Frequency is required - then parameter valid is false when Frequency is empty. When I enter a value for Frequency, valid is true. This seems like 'valid' is indicating that the condition is met. I think this doesn't tell me the Frequency is a required input.

jwaliszko commented 7 years ago

OK, I was a bit confused, but I understand now. Unfortunately, currently the eavalid only gives you the result of the requirement verification, not the result of the condition, based on which the requirement is being verified.

jwaliszko commented 7 years ago

Looking at the implementation, you're interested in the result of this line only, instead of the result of the entire function. The thing is, for the requirement verification, the condition expression is checked only if the field is empty - if it is not empty, the verification is redundant since field value is already given (even from performance perspective computing of the expression is a waste of time in such a case).

Your use case is quite reasonable though, and I haven't considered it before. Now, I cannot think of any simple help here, despite that one you've mentioned - computing the expression by yourself which is a bit tricky.

Maybe we could define some kind of a flag in the settings, e.g. resolveExpressionsAlways, and even if value is provided, requirement expression is computed (and maybe even, for the sake of cohesion, if value is not provided, assertion condition is computed as well), and passed to eavalid as the next parameter.

jwaliszko commented 7 years ago

I think your use case is the same as already defined in the following issue: https://github.com/jwaliszko/ExpressiveAnnotations/issues/121.

rickparrish commented 7 years ago

Yes, it does sound very similar to what I was asking in #121. And I think the same solution would work -- when the 'eadisplay' event is raised the page could handle that and show/hide the askterisk as necessary instead of showing/hiding the whole question as we were doing.

We made additional customizations to EA before putting it into use, so I don't remember for sure whether those changes were sufficient on their own, or if we ran into problems and had to make further adjustments though.

jwaliszko commented 7 years ago

@trenzer, @rickparrish: With the commit https://github.com/jwaliszko/ExpressiveAnnotations/commit/304721ac4a890e7088310c8387a9f3608bd0851e I've proposed some changes. If you'd like to test it, please fork the repo, uncomment 2 lines of Home.cshtml, mainly 60 and 149, and give it a try. I'm also opened for any more elegant or more generic solutions.

Thank you for reporting these issues.

michielvanerp commented 7 years ago

Thanks for the effort and quick thinking! I'm a github newbie so will try that, probably tomorrow.

jwaliszko commented 7 years ago

N/P, you're welcome. It may be funny, but in order to make it clear for future of what this issue is about (mainly for myself ;), truth table is given below:

[RequiredIf('cond')]
datatype? Field { get; set; }
field value provided cond valid
0 0 1
0 1 0
1 0 1
1 1 1
jwaliszko commented 7 years ago

Change is shipped with version 2.9.2 (published yesterday), and respective examples are built into the web sample project (entry point here).