GoogleWebComponents / google-recaptcha

Google reCAPTCHA element
19 stars 19 forks source link

demonstrate usage inside a form #11

Open jab opened 8 years ago

jab commented 8 years ago

The demo at http://cbalit.github.io/re-captcha/components/re-captcha/ (click "DEMO" in the upper right) demonstrates different flavors of the captcha (default, dark, and audio), but they're all shown in isolation, i.e. not in the context of a containing form, which is the most common place captchas are needed.

Would you consider adding an example to the demo of a google-recaptcha inside an iron-form, fully integrated into the form's overall validity state? (Bonus points for a paper submit button whose disabled state is bound to the form's validity.)

Figuring out how to adapt a default recaptcha to dark or audio is straightforward to newcomers, but figuring out how to hook it into an iron-form isn't, so it'd be a great improvement to the demo to show this.

Thanks for your consideration and for the great work on Polymer.

jab commented 8 years ago

Just to give a little more background, here is iron-form's validate function:

    validate: function() {
      var valid = true;
      // Validate all the custom elements.
      var validatable;
      for (var el, i = 0; el = this._customElements[i], i < this._customElements.length; i++) {
        if (el.required && !el.disabled) {
          validatable = /** @type {{validate: (function() : boolean)}} */ (el);
          // Some elements may not have correctly defined a validate method.
          if (validatable.validate)
            valid = !!validatable.validate() && valid;
        }
      }
      // Validate the form's native elements.
      for (var el, i = 0; el = this.elements[i], i < this.elements.length; i++) {
        // Custom elements that extend a native element will also appear in
        // this list, but they've already been validated.
        if (!el.hasAttribute('is') && el.willValidate && el.checkValidity && el.name) {
          valid = el.checkValidity() && valid;
        }
      }
      return valid;
    },

When a <google-recaptcha> is in an iron-form, it does not get added to _customElements automatically, so its validity is not checked in the first loop. Then, when the second loop attempts to iterate over the form's native elements, it ends up including the <textarea id="g-recaptcha-response"...> in the recaptcha's shadow dom, whose checkValidity() method is called and appears to be returning true even when the puzzle has not been solved. WTF?

cbalit commented 8 years ago

Making the captcha works with iron-form need some work because it needs to implement IronFormElementBehavior. Anyway i've updated my demo file to show usage in a form with some basic javascript calls. You can' find it here The gh-page is not updated because i have some trouble with gp.sh script. Hope it will help.

jab commented 8 years ago

Hi @cbalit, thanks so much for taking a look. Indeed, I'm specifically asking about deeper integration with iron-form (via IronFormElementBehavior and IronValidatableBehavior), so just demoing how to enable a submit button imperatively on successful captcha response doesn't help with demonstrating a live binding of the captcha's validity state to the form's overall validity state (captcha invalid -> form invalid; captcha and all other validatables valid -> form valid). Is implementing this deeper integration very tricky? I'm super new to Polymer but happy to take a crack at a PR if you can provide guidance and review.

cbalit commented 8 years ago

@jab I don't think it will be tricky to implement those behaviours. I will be happy to review any PR even if I'm not a crack :). Feel free to ask if you have any questions or remarks.

jab commented 8 years ago

Core devs, please see the PR I just submitted for this -- #12 -- and let me know what you think!

cbalit commented 8 years ago

@jab i look at your PR and it looks great.

Good job

Cyril

jab commented 8 years ago

Thanks for reviewing @cbalit! I just added a few more commits and responded to your comments over at #12 -- should we move discussion there?