Closed bassplayer7 closed 8 years ago
I couldn't test the plugin in detail, because right now and in the next couple of months I will be mostly away from the computer, but I really like the changes you did so far. I had planned to simplify the plugin in the near future and get rid of the things that currently don't make much sense (like how I handle the language and config settings and things you changed :) ), but I didn't have much time. I really appreciate your contributions.
I understand - thanks for writing the plugin! I really like having the Google Recaptcha on these forms. Even if a honeypot does most of the work, this 2nd gen Recaptcha is excellent.
I hope to be able to contribute more as I continue to use the plugin on sites.
As I was working with the plugin, I noticed that it duplicated a
<section />
tag which I thought was unusual. I looked into the code and found that the plugin was appending the template onto the page's current content. That made sense because I had noticed that I didn't have to add anyinclude
statements in my theme's templates. However, with that, it seems to be a non-standard and non-adjustable way of adding the form.I developed the module to be backwards compatible, while adding an option to disable the default function of appending the template. As a result, users can now easily disable that and add their own
include
tag in their templates. I believe that this is a superior way to do it because it allows easier and faster customization and seems to be more in line with what other plugins do (like Pagination and such).Also, I split out the actual form into it's own file, separate from the markup that surrounded it. The advantage of doing it this way is that users don't have to override the entire form to make a change to the form's container. I moved away from using an ID to style the form. An ID is one of the "strongest" selectors and as a result it is hard to override. I understand that it is actually considered best practice to never use an ID as a CSS selector (obviously JS is a good use for IDs). Using a custom class allows developers to easily override the styles without having to completely disable them.
I split the form messages out into their own method and property. This allows them to be globally accessible (within the class's methods). That makes it very easy to pass them into Twig and let the template handle the rest. The clear benefit of this is being able to style all of the messages very simply. By having the messages in their own file, developers can easily override them if they want to.
I developed on the
dev
branch, but checked out a new branch to allow greater control of what you merge, if you like these changes, or not. Please feel free to ping me with any questions or comments: I'm more than happy to collaborate more on this or explain any of the code that I changed. I really like this plugin and hope these changes will encourage more users.I did not test this with PHP 5.3, but am not aware of anything that would cause it to break. I also wasn't sure what template it is designed to drop into. As a result, I didn't minimize any of the markup that surrounded the form. Again, if I missed something, please let me know and I will work on it.
Possible Future Improvements The one other thing that I thought about changing was the
mail()
function and moving to Twig and SwiftMailer for that.