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

Update 'autoStoreData' feature to support object notation input names #385

Closed
wants to merge 7 commits into from
Closed

Update 'autoStoreData' feature to support object notation input names #385

wants to merge 7 commits into from

Conversation

paulmsmith
Copy link
Contributor

Sometimes it's useful to utilise URL encoding in the name of form inputs when routing. For example:

<form method="post" action="/">
    <input type="text" name="user[name]">
    <input type="text" name="user[email]">
    <input type="submit" value="Submit">
</form>

Using user[name] as the value for the input's name attribute means that in ExpressJS when using bodyParser (like the prototyping kit) we can do the following in a routes file:

router.post('/', function(request, response){
    console.log(request.body.user.name);
    console.log(request.body.user.email);
});

user[name] becomes user.name but the autoStoreData feature doesn't account for that.

The pull request should deal with that. Removing _unchecked from the created objects that get sent back when using 'urlencoded' input names described above and allowing the 'checked' method to deal with those input names.

This is a starter. Feedback and improvements welcome but this gets the ball rolling.

@paulmsmith
Copy link
Contributor Author

Bah.. forgot to lint. Will fix now.

@paulmsmith
Copy link
Contributor Author

Calling on reviewers.. cc: @joelanman @edwardhorsford ;)

@joelanman
Copy link
Contributor

thanks for this work @paulmsmith however it looks like a bit of a complex change, so it might take a while for someone to be able to review it. As ever, the kit does not actually have a team assigned to it.

@paulmsmith
Copy link
Contributor Author

Yes, no worries @joelanman.

@joelanman
Copy link
Contributor

Sorry for the delay on this!

Going to close as it is out of date with the latest code (our fault)

I think this is an updated version of this work:

#573

@joelanman joelanman closed this Aug 8, 2018
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