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

vue refactor (remove interval loading, fix prop definitions and use $ref and $emit event) #21

Merged
merged 2 commits into from Jan 25, 2019

Conversation

Fluxlicious
Copy link
Contributor

@Fluxlicious Fluxlicious commented Jan 21, 2019

  • Refactored the Vue component to load a lot faster (not using intervals anymore)
  • Made it possible to bind the response token to a model, the Vue way.
  • Made all props optional with default values that can be configured on the component

Upgrade warning: this upgrade breaks functionality. Prop changed from elementId to id. Adjust your code accordingly.

Note: when using model binding you need to call execute() after each request to get a new token.
In your template:

 <google-recaptcha-v3 ref="captcha" v-model="grecaptcha"></google-recaptcha-v3>

In your script:

this.form.post('/post-url')
    .then(response => {
        this.$refs.captcha.execute();
    })
    .catch(exception => {
        this.$refs.captcha.execute();    
    });

…als anymore)

- Made it possible to bind the response token to a model, the Vue way.
    <google-recaptcha-v3 v-model="yourDataAttribute"></google-recaptcha-v3>
- Made all props optional with default values that can be configured on the component
@RyanDaDeng
Copy link
Owner

Wowow!!! I was planning to refactor it as well. Wow! I will check it tonight as soon as possible.

@RyanDaDeng RyanDaDeng self-assigned this Jan 22, 2019
@RyanDaDeng RyanDaDeng added the enhancement New feature or request label Jan 22, 2019
@RyanDaDeng
Copy link
Owner

RyanDaDeng commented Jan 22, 2019

Hello @Fluxlicious , I have played it a bit around with the proposed approach, I am unable to render multiple recaptcha on the same page e.g. (probably I am doing something wrong)

    <div>
        <form method="POST" action="/verify">
            <div>
                <label for="exampleInputEmail1">Email address</label>
                <input type="email" class="form-control" id="exampleInputEmail1"
                       aria-describedby="emailHelp"
                       placeholder="Enter email">
            </div>
            <div>
                <label for="exampleInputPassword1">Password</label>
                <input type="password" id="exampleInputPassword1"
                       placeholder="Password">
            </div>
            <div>
                <div id="contact_us_id"></div>
            </div>
            <div>
                <div id="contact_us2_id"></div>
            </div>
            <button type="submit">Submit</button>
        </form>
        <google-re-captcha-v3
                :siteKey="this.siteKey"
                :id="'contact_us_id'"
                :inline="true"
                :action="'contact_us'">

        </google-re-captcha-v3>

        <google-re-captcha-v3
                :siteKey="this.siteKey"
                :id="'contact_us2_id'"
                :inline="true"
                :action="'contact_us'">

        </google-re-captcha-v3>
    </div>

errors:
Uncaught Error: reCAPTCHA has already been rendered in this element at Object.N1 [as render] (VM667 recaptcha__zh_cn.js:532) at VueComponent.render

Yeah, I might need to look into it deeply this weekend and also adding some unit tests later for the vue component later this week.

I really appreciate the work you have done! The proposed approach is good, I need to ensure some edge cases to be verified as well, I will get back to you soon. Will be no later this week :)

@RyanDaDeng RyanDaDeng changed the title master vue refactor (remove interval loading, fix prop definitions and use $ref and $emit event) Jan 22, 2019
@Fluxlicious
Copy link
Contributor Author

No you're right, didn't check that. Also having multiple recaptcha tried to load the script more than once as while the script wasn't loaded yet. Fixed it in commit f97251d by identifying the presence of the script tag and while the script is still loading push the render callbacks to an array and execute them all once the script is loaded.

@RyanDaDeng
Copy link
Owner

@Fluxlicious Sorry for the late reply. I am looking into it now.

@RyanDaDeng RyanDaDeng merged commit 7812ad9 into RyanDaDeng:master Jan 25, 2019
@RyanDaDeng
Copy link
Owner

good good, also I learnt some from your approach. 👍

@RyanDaDeng
Copy link
Owner

I have sent a contributor inv to you, feel free if you want to accept the invitation. 🥇

@xalunda
Copy link
Collaborator

xalunda commented Jan 25, 2019

Awesome refactoring @Fluxlicious !
I would recommend adding a warning on the release note as it is not backwards compatible:

  • >= 2.2.0 => prop changed from elementId to id (much better 👍)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants