Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

more setting options #159

Open
1 of 4 tasks
GlauberF opened this issue Nov 27, 2019 · 2 comments
Open
1 of 4 tasks

more setting options #159

GlauberF opened this issue Nov 27, 2019 · 2 comments

Comments

@GlauberF
Copy link

Summary

I'm submitting a:

  • bug report
  • feature request
  • question / support request
  • other

Description

I can use more options, something like this below.
set language, thema, etc ...

   [siteKey]="siteKey"
    (reset)="handleReset()"
    (expire)="handleExpire()"
    (load)="handleLoad()"
    (success)="handleSuccess($event)"
    [useGlobalDomain]="false"
    [size]="size"
    [hl]="lang"
    [theme]="theme"
    [type]="type"
    formControlName="recaptcha"
@DethAriel
Copy link
Owner

Hi @GlauberF ! Theoretically, all of these should be possible, but below I take a closer look at these options. I do hope that some of the thoughts I express below will help you (and other readers) gain insight into how I'm thinking about those.

Supported inputs

The (+) part delineates an option that wasn't mentioned in your original request.

  • siteKey
  • size
  • theme
  • type
  • badge (+)
  • tabIndex (+)
  • errorMode (+)
  • (error) (+), to be renamed in a future release due to conflict with Angular-native output name.

Supported form directives

  • formControlName
  • formControl
  • ngModel

Sort of supported options

  • useGlobalDomain. Currently, it's supported via RECAPTCHA_BASE_URL injection token that RecaptchaLoaderService relies on. Why is it not a part of <re-captcha> component public API? The loading of the reCAPTCHA assets is supposed to be bringing in reCAPTCHA functionality to the entire application. I cannot imagine a use-case where one would load reCAPTCHA from the global domain in one part of an app, but then would proceed with loading it from a different domain in another application section - that simply makes little sense from the performance perspective (additional latency for script loads, increased landscape for network failure modes, larger memory footprint, etc). Thus, the script loading configuration is treated as an application-wide configuration setting.
  • (load). This one is actually supported, but again, not on the <re-captcha> level, but on the RecaptchaLoaderService level instead. One can rely on the RecaptchaLoaderService.ready Observable to figure out when the assets have been loaded. Again, this seems to make sense if you factor in the "global" nature of asset fetching from above. That isn't the way for reCAPTCHA v3 though, but v3 is a separate subject altogether, and I imagine your original proposal doesn't relate too much to v3.

Event emitters

  • (reset) + (expire) + (success). Those are combined into one event emitter - (resolved). When (resolved) emits a non-falsy value, it is equivalent to (success), otherwise it's one of (reset) or (expire). Today, I don't see a use-case that would warrant the ability to distinguish between the two. Indeed, when recaptcha expires, the checkbox (v2) version of it conveys this information to the user, and whenever the developer resets it (either by invoking .reset() directly or manipulating the value through a form controller) - they already know about that action, don't they?
    Ultimately, this is a trade-off between the simplicity of use and power. In this particular case, I don't consider the additive value of power to warrant the degraded API ease-of-use.

Options to be supported in future

  • edition: "standard" | "enterprise" (global). This is pretty straightforward - a way to support enterprise reCAPTCHA edition in a future-proof way (in a sense that reCAPTCHA might add smth like a "premium" version in future)

Unsupported options

  • hl / lang. The main reason support for this option has been lagging is actually quite simple - there is no public API support in reCAPTCHA core for changing the language dynamically. There was an attempt to add support for this in refactor(component): add the ability to change the lang dynamically #93, yet this PR was ultimately declined, and my reasoning has not changed since then.

@WalkerWalker
Copy link

edition: "standard" | "enterprise" (global). This is pretty straightforward - a way to support enterprise reCAPTCHA edition in a future-proof way (in a sense that reCAPTCHA might add smth like a "premium" version in future)

I wonder if the current library supporting google reCAPTCHA enterprise?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants