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

Simplify field validation #100

Merged
merged 1 commit into from
Apr 11, 2017
Merged

Simplify field validation #100

merged 1 commit into from
Apr 11, 2017

Conversation

OleVik
Copy link
Contributor

@OleVik OleVik commented Feb 23, 2017

As noted in #99, clicking the button will allow the form to be submitted. This seems to be because event.preventDefault(); will not work properly when declared multiple times. For semantics and accessibility I wrapped the input- and button-tags in a form-tag, explicitly declared a button-type, and check the submission against a simple onSubmit.

It remains pure JavaScript, but is comparatively simpler in that it needs no addEventListener for keyup or onclick, only onsubmit. HTML-structure is slightly simplified for readability. Tested with Grav v1.1.18 (from source) and plugin v1.10.0. @core77, mind testing it?

Solves getgrav#99, simplifies structure and method.
@core77
Copy link

core77 commented Feb 23, 2017

@OleVik Works in Chrome and IE!

In Firefox:
There occurs a ReferenceError: event is not defined (validateSearch onsubmit)
The URL changes from search/query:myquery (Chrome, IE) to ?searchfield=myquery (Firefox)

Edit: When you pass in the 'event', the error doesn't occure. I commented your file...

}, false);
}

<form name="search" onSubmit="return validateSearch();">
Copy link

Choose a reason for hiding this comment

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

Should be return validateSearch(event);


</form>
<script>
function validateSearch() {
Copy link

Choose a reason for hiding this comment

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

Should be function validateSearch(event) {

@flaviocopes flaviocopes merged commit ad97a0b into getgrav:develop Apr 11, 2017
@flaviocopes
Copy link
Contributor

@core77 thanks for testing and mentioning that, fixing after merging

flaviocopes added a commit that referenced this pull request Apr 11, 2017
@OleVik OleVik deleted the patch-1 branch April 11, 2017 12:57
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.

3 participants