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

fix bug, when formatSubmit was improperly appending suffixes #342

Closed
wants to merge 1 commit into from

Conversation

dylanjha
Copy link

  • for example a name like order[date] would incorrectly be changed
    to order[date]_submit
  • in this example, the input should have the name order[date_submit]

* for example a name like `order[date]` would incorrectly be changed
to `order[date]_submit
* in this example, the input should have the name `order[date_submit]`
@dylanjha
Copy link
Author

@amsul if you like this change, I can write a test for it. figured I would put in the PR first before I spend more time on it 😃 . thanks for maintaining this library, I'm diggin it! 👍

@amsul
Copy link
Owner

amsul commented Apr 12, 2014

I really appreciate the contribution and kind words :)

I apologize for the delayed response with this. But now that I look at it, it seems it’s trying to solve the same problem that led to the introduction of the hiddenPrefix option: #209.

And now with the suggestion to remove the visible input’s name and use that as the hidden input’s name (#377), it makes me feel like this is no longer necessary.

In my opinion, I agree with @psorowka about the server not needing to be aware of changes by a 3rd party client-side plugin. The vice-versa should be true as well.

If this hidden prefix is causing any trouble, it can just be set to false and everything should sail smoothly. I’d like to hear any counter thoughts you might have on this.

Cheers!

@amsul amsul modified the milestones: v3.4.2, v3.4.1 Apr 13, 2014
@amsul amsul closed this Apr 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants