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

only submit form on input #804

Merged
merged 2 commits into from
Jun 11, 2017
Merged

Conversation

chongma
Copy link
Collaborator

@chongma chongma commented Jun 8, 2017

#803 small syntax error

@ggam
Copy link
Collaborator

ggam commented Jun 8, 2017

Just to undertand, how does this change the behavior? How will this work from now?

@chongma
Copy link
Collaborator Author

chongma commented Jun 8, 2017

it will run the default command defined by b:defaultCommand when pressing return on an html input. without the colon if the return key is pressed anywhere in the form, especially b:inputTextarea the form would be submitted. it is the desired behaviour i believe. just a syntax error in the jquery

@chongma
Copy link
Collaborator Author

chongma commented Jun 8, 2017

also i submitted a PR to show a b:inputTextarea on the showcase to demonstrate that it works correctly

@ggam
Copy link
Collaborator

ggam commented Jun 8, 2017

Thanks! I also think that's what is expected. I just wanted to be sure.

Btw, how would control+enter behave? Would that trigger the submit from the text area? That should work in my opinion.

@chongma
Copy link
Collaborator Author

chongma commented Jun 9, 2017

could add another line to check for html textarea and use check described in this stack overflow https://stackoverflow.com/questions/1684196/ctrlenter-jquery-in-textarea. i have updated the PR to reflect this

@TheCoder4eu
Copy link
Owner

@chongma , seems ok to me, can I merge it?

@chongma
Copy link
Collaborator Author

chongma commented Jun 11, 2017

Yes please. I tested it but wanted a second opinion before merging

@TheCoder4eu TheCoder4eu merged commit d25c25d into TheCoder4eu:master Jun 11, 2017
@TheCoder4eu
Copy link
Owner

Ok, thanks, I merged it!

@TheCoder4eu TheCoder4eu added this to the v1.1.2 milestone Jun 11, 2017
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