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

fix hard-coded form link #20

Closed
wants to merge 3 commits into from
Closed

fix hard-coded form link #20

wants to merge 3 commits into from

Conversation

jeffhx
Copy link

@jeffhx jeffhx commented Jun 22, 2021

this fix uses the config value for the contact form

Copy link
Owner

@kdevo kdevo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate your attempts here, but your PR is not needed. I've left a few comments to clarify why.

Nevertheless, it was not all for nothing: I've taken note that I need to better document the honeypot functionality and created #23 to improve the overall documentation. Feel free to contribute to this issue!

@@ -9,7 +9,7 @@ <h1><span class="icon icon-mail"></span>{{ i18n "letsChat" }}</h1>
</div>
</div>

<form id="form-contact" action="https://formspree.io" method="POST">
<form id="form-contact" action="{{site.Params.Feat.ajaxBasin}}" method="POST">
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not hardcoded. Instead, it is not used at all and meant to trick spammers (see my comment below). The AJAX Request is handled via JS. Please take a look here:

var email = $('input[name=email]').value
// AJAX request
var request = new XMLHttpRequest(),
data = {
name: $('input[name=name]').value,
_replyto: email,
email: email,
_subject: $('input[name=_subject]').value,
message: realmsg.value,
}

@@ -15,7 +15,9 @@
{{ end }}
{{ if or (.Site.Params.credit) (eq (isset .Site.Params "credit") false) -}}
<div class="col-xs">
<small>Theme: <a href="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/kdevo/osprey-delight" target="_blank" rel="noopener">Osprey Delight</a></small>
<!--
<small>Theme: <a href="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/jeffhx/osprey-delight" target="_blank" rel="noopener">Osprey Delight</a></small>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me it's a bit discouraging to not receive any credit at all.
Neverheless, I've left it completely configurable: You can set the .Params.credit in your site's config to false and the attribution will disappear (take a look at the condition a few lines above).

I think I've kept the attribution very short and unobtrusive already, so I'm wondering: What's the reasoning behind removing it completely?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello Kai, thanks for the reply. I was not aware of this configure, thanks for pointing it out.
The reasoning removing it completely is i plan to use it in a production site.

Sorry this discourages you in any way. It was not my intent.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem, I understand that it was not your intent. Thanks for your answer!

<div class="col-xs-12">
<textarea name="message" aria-label="Please enable JS" required></textarea>
<textarea name="message" aria-label="Please enable JS" ></textarea>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is part of the honeypot that is meant to attract spammers, as described under "Spambot protection" in the README. I think you've also fallen into the honeypot here by thinking it's the actual message field.
That's my mistake, as I did not leave a comment here to highlight that this is handled via JS instead by manipulating the DOM so that message2 is the actual message field:

var realmsg = $('textarea[name=message2]')
// For spam protection, we use "message" as a honeypot field:
var honeypotmsg = $('textarea[name=message]')
if (realmsg === null) {
return;
}
setVisibility(realmsg, true)
setVisibility(honeypotmsg, false)
honeypotmsg.removeAttribute("required")

Sorry for that, I'll leave a comment above to clarify it.

@kdevo
Copy link
Owner

kdevo commented Jun 23, 2021

Let's continue the discussion at #22.
Thanks!

@kdevo kdevo closed this Jun 23, 2021
@kdevo kdevo mentioned this pull request Feb 2, 2022
kdevo added a commit that referenced this pull request Apr 30, 2022
Although not a bug, it lead to confusion multiple times, see e.g.:
- #28
- #20

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

Successfully merging this pull request may close these issues.

None yet

2 participants