-
Notifications
You must be signed in to change notification settings - Fork 142
Contact form plugin #2191
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
Contact form plugin #2191
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and the form works smoothly for me! Nice implementation 🎉
Mostly nits on documentation!
The form submissions emails ended up in junk/spam for me - not sure if adding a note in the UG on checking spam folders would be suitable?
docs/userGuide/plugins/web3Form.md
Outdated
|
|
||
| This plugin allows you to create forms whose response will be sent directly to your email, using the [Web3Form](https://web3forms.com/) API. | ||
|
|
||
| To set it up, get an access key from [Web3Forms](https://web3forms.com/). Then add`web3Form` to your site's plugin, and add the `accessKey` parameter via the `pluginsContext`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yucheng11122017 I assume this access key is not a 'secret' key?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Urm not exactly? It's linked to the user's email. web3forms uses it to redirect the form submissions to the user's email.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there potential for misuse of the key? My current understanding is that if someone wants to abuse this key, they could use it to direct the submissions from a different form to the user's email address, but this would have the same effect as simply spamming junk submissions to the original form so it doesn't give a malicious user any additional tool for abuse.
Unless the key can be used to reveal the user's actual email (which the user might have intended to keep private)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be fine, if the key is not meant to be a 'secret' key that should not be made public.
|
Thank you for reviewing the PR @jovyntls :)
I have added a warning to the documentation. Also added a warning that they are limited to 250 submissions per month if they are using the free plan |
tlylt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @yucheng11122017 for the PR. Left some comments. Also, I think the suggestions from Jovyn are not yet resolved (FYI, in case you have overlooked them).
Hi @yucheng11122017, I just discussed with @jovyntls, I think if we can change the “contact us" default from h2 to a div with the relevant bootstrap classes to style it, it would prevent the issue from occurring. What do you think about this? |
Yeah ok sounds good! There is nothing that disables the page-nav from rendering some headings right haha |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks again @yucheng11122017
Yeah ok sounds good! There is nothing that disables the page-nav from rendering some headings right haha
Nope as far as I know! Might be a possible feature request but I don't think there's much utility to doing so (especially since using Bootstrap styles would be a possible quick workaround for authors who wish to do so)
(Edited the PR description to close the original issue with keywords!)
|
Hi @tlylt, thanks for bringing that up! I changed the selector to only affect the inputs in the web3Form. |
jovyntls
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't catch this earlier (my bad)!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last nit, can we use the newly introduced node types (from #2201 - MbNode, NodeOrText etc) wherever possible instead of cheerio.Element and DomElement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in ac99277! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @yucheng11122017, just want to share that besides changing the node type, there might be a need to adjust the code accordingly. So in this case, some of the lines do not make sense anymore:
e.g. these few lines are not needed anymore because attribs will always exist

In the future do take note of the general lesson here as well: the review comment may be specific and points out one thing, that doesn't mean other relevant code changes will not be needed. So relooking at what other potential changes are necessary would help a lot.
@jovyntls will make a follow-up update soon, so no need to make further changes here
jovyntls
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!! 🥳🥳🥳
tlylt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yucheng11122017 Try merging this (note that this is not typescript PR, so it should be a squash commit strategy)

What is the purpose of this pull request?
Closes #2095
Overview of changes:
Add a web3Form plugin to allow users to create forms which uses the web3Form api
Supports
Anything you'd like to highlight/discuss:
In terms of number of submissions per month, web 3 form free plan seems to be the best so far with 250 submissions per month. However it only allows one email target vs Formbold with 2 email target but 100 submission / month.
For now, I think web3form offers the best free plan.
Testing instructions:
Proposed commit message: (wrap lines at 72 characters)
Add web 3 form pluginChecklist: ☑️