Skip to content
This repository has been archived by the owner on Feb 2, 2023. It is now read-only.

docs(website): contact form #250

Merged
merged 3 commits into from
Jun 22, 2016
Merged

docs(website): contact form #250

merged 3 commits into from
Jun 22, 2016

Conversation

Shipow
Copy link
Contributor

@Shipow Shipow commented Jun 22, 2016

No description provided.

@algobot
Copy link
Contributor

algobot commented Jun 22, 2016

By analyzing the blame information on this pull request, we identified @LukyVj and @vvo to be potential reviewers

function validateForm() {
var x = document.forms["form-contact"]["first_name"].value;
if (x == null || x == "") {
alert("Name must be filled out");
Copy link
Member

Choose a reason for hiding this comment

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

It worth what it worth, but do we really want to display this in an alert ? Seems a bit too much no ?
An error message over the input would feel less aggressive to me.

My 2cts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not the latest commit, it was a draft :)

@redox redox force-pushed the feature/contact-form branch from e1cfd9c to dc0d471 Compare June 22, 2016 19:53
@redox redox merged commit 04be137 into master Jun 22, 2016
@redox redox deleted the feature/contact-form branch June 22, 2016 19:59
@redox
Copy link
Contributor

redox commented Jun 22, 2016

gg @Shipow 👍

@vvo
Copy link
Contributor

vvo commented Jun 22, 2016

NICE

@vvo
Copy link
Contributor

vvo commented Jun 22, 2016

quick tip: when merging you can decide to merge commits into a single commit, here would have been useful to you

@vvo
Copy link
Contributor

vvo commented Jun 22, 2016

All from github merge button

@vvo
Copy link
Contributor

vvo commented Jun 22, 2016

Ok this is what you actually did :D

@redox redox mentioned this pull request Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants