enthusiastic-js / form-observer

A simple utility for reacting to events from a form's fields
MIT License
7 stars 0 forks source link

feat: Enable Devs to Focus (and Scroll to) Fields that Fail Validation (`FormValidityObserver`) #5

Closed ITenthusiasm closed 1 year ago

ITenthusiasm commented 1 year ago

You can see the commit message for the details. Because this commit included a significant design decision, we're opening this PR for historical purposes.

Key Design Decision: Deprecate the FormValidityObserver.validateFields(names: string[]) Overload

Originally, when we were first thinking through the designs of the FormValidityObserver, the validateFields(names) overload was a convenience function. At the time, we were already intending to have validateFields loop over all of the "registered" (now configured) field names. So we figured, "Why not let the developers loop over a subgroup of those fields"?

Well... Things have changed. And the biggest change on this front is that the FormValidityObserver no longer requires a field to be configured for it to participate in validation. Supporting new features -- such as focusing the first field that fails validation in a form -- has made it more difficult to support validating all of a form's fields (1st overload) and validating a specific set of named fields (2nd overload) simultaneously. Given that we don't want this additional complexity in the FormValidityObsever, given that the validateFields(names) overload isn't likely to be used often, and given that the overload can be written rather easily in userland, we are now deprecating validateFields(names) to make our own codebase more maintainable. Here's the justifification:

In a situation where a developer would run

formValidityObserver.validateFields(["first-field", "second-field"]);

they could just as easily run

["first-field", "second-field"].forEach((name) => formValidityObserver.validateField(name, options));

Admittedly, the latter option is slightly more verbose than the former option. However, both options can be written easily in 1 line. And the latter option is technically more explicit (because any reader of the code immediately knows exactly what's happening).

Since the developer knows which combinations of fields include asynchronous validators, they can easily handle the Promise-based version as well:

const names = ["first-field", "second-field"];
await Promise.allSettled(names.map((name) => formValidityObserver.validateField(name, options)));

Again, this is more explicit, and it's very simple. So we don't think that dropping support for the validateFields(names) overload will actually be disadvantageous for anyone. We feel comfortable dropping support for the overload in light of this.