I'm not a big Php guy so I can't be too thorough in this section. The biggest thing is that you seem to be writing ... well ... a php script. While that absolutely works it's incredibly old-school and doesn't do anything as far as guiding you into the pit of success. So first bit of advice: use a framework. At time of writing, the two I've been hearing most buzz about are Laravel and Codeigniter. This is constantly changing but just pick one which you've heard about and use it, that will help you learn enough concepts to make an educated choice next time you need to select one.
Second, at the bottom you seem to have some invalid php. You have some missing braces in that if statement toward the bottom and it has two
else clauses in a row. I'm not sure if that will cause an error but it will definitely never run the third clause.
Finally, your design leaves open a huge security hole if the site is ever opened to the public. You accept the message as an input parameter, much worse, you accept the email. Think of what happens when someone does a query to
yoursite.com/yourpage.php?email=yourgrandma&message=Send%20me%20all%20the%20moneyand again, and again. Spammers love finding unsecured services like this!
A much better approach is to have the emails (and maybe message) stored on the server already in the database or a text file. Then you could supply parameters like
email=1 indicating "email the person with id=1".
Having this behind a login page is not a (good) defense either. Someone could easily put the above url in the src of an
<img> element and every time someone who had recently logged into your site visits a page they put that on (like a comment or forum page) you will be sending spam! To defend against this make sure that page does not accept the Http GET verb (in this situation POST would be appropriate). This is exactly the kind of error that a framework shoudl help defend you from.
Finally, I see what you're doing with the ajax flag but that's not the typical thing to do. Whether the operation succeeded or not should be determined by the returned Http code (200 for success and maybe 400 for error). Whether you should be returning json or html should be determined by a concept called content negotiation. This is again, something that your framework should be handling for you.
name is not a valid attribute for the
contactForm elements on the page at once? What if you have 2 offices and each one wants its own contact form? What about elements with
id=submit? A better approach I think in this case is to use a class. That is not to say that you should never use ids but in most cases the benefit isn't worth the risk.
Next, you don't technically need
<button type=submit>, it's optional but submit is the default behavior.
Finally, I'm not 100% sure of the spec for
<label> but I don't think it works by associating the label with the following
<input>. Instead you have to use the
for attribute and
ids (which I've already argued is bad). Or do what I recommend, something like:
<span class="label">Email Address</span>
<input id="email" type="email" placeholder="Enter Email" required>
camelCase with no underscores. It's not a big deal but it's pretty standard. The only exception is to use
PascalCase for variables that are intended to be used with the
new keyword (which you don't have here).
In my opinion you are over-specifying your selectors, no need to do
input[id=foo] when id is already specified as unique, no need to
span holds no semantic nor structural meaning. Consider that all this stuff is indexed in the browser so by adding more selectors you might actually be slowing things down however slightly.
You don't need to use
$('id=something') to select, you can just say
Your scope management looks pretty good, I'm happy to see you using
find - another syntax which achieves the same purpose and you might find flows better (or not) is
Finally, just about everything you're doing in js is handled in a very elegant manner by the jquery.form plugin. Just use that.