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

Support old values for nested form fields #61

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

imacrayon
Copy link

This PR respects old form input values even if the input is named using nested "array notation". Old values for form inputs with names like profile[email] and employees[0][first_name] will work just like any other field.

I also added array notation support to the <x-error> component so that it is symmetric with form inputs:

<x-error field="cars[1][model]"/> or <x-error field="cars.1.model"/> both fetch the same error messages now.

@driesvints
Copy link
Member

This is a very neat PR, thanks! I'm wondering if we should format the ID's for array named components differently because I don't know if using [] in id's is semantically correct from an HTML perspective?

Also, I'm not sure about adding the new dotName property. Maybe there's an opportunity to create a helper which we can use throughout the app?

@imacrayon
Copy link
Author

imacrayon commented Sep 9, 2020

I don't know if using [] in id's is semantically correct from an HTML perspective?

The HTML5 spec is surprisingly lax, it seems that the only restriction is that the id must contain At least one non-control character. That said, it would be easy enough to snake case the id too.

Maybe there's an opportunity to create a helper which we can use throughout the app?

That's a great idea, the dotName property is a little messy. I'll get that changed.

@imacrayon
Copy link
Author

imacrayon commented Sep 9, 2020

@driesvints any preference on what this "dot name" helper should be named? I also considered using "session key".

@driesvints
Copy link
Member

That said, it would be easy enough to snake case the id too.

Yeah I agree.

any preference on what this "dot name" helper should be named?

Heh can't think of anything myself atm. Probably something not too long.

@imacrayon
Copy link
Author

I took a stab at refactoring the dotName property into a helper. I landed on adding a Str::dot() macro to Illuminate\Support\Str does that feel ok or should we just make our own standalone helper instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants