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

Nested fields in the entry editor aren't validated #467

Closed
erquhart opened this issue Jun 29, 2017 · 11 comments · Fixed by #1873
Closed

Nested fields in the entry editor aren't validated #467

erquhart opened this issue Jun 29, 2017 · 11 comments · Fixed by #1873

Comments

@erquhart
Copy link
Contributor

Field validation only works on the top level of widgets in the entry editor, fields within objects or lists aren't validated at all.

The primary blocker here is that fields are referenced by name internally, which would then require nested fields to know the names of each ancestor field, or that their state match the nested hierarchy of the fields. Both approaches are anti-patterns.

To move forward, field state handling should be refactored to use unique id's. This will likely require a mechanism for translating the nested data in the config file to new id's at load time.

@erquhart
Copy link
Contributor Author

erquhart commented Dec 7, 2017

I'm removing Priority tags everywhere, but this one's a very big deal.

@ArcaneTSGK
Copy link

Any idea on a timeframe on this one? Related: #1873

@lmcorreia
Copy link
Contributor

@erquhart I've been some time away from Netlify CMS but the handling of the nested fields seems to be roughly in the same state so I reckon I remember how it's being done. Mind if I pick this up or do you folks have any larger plans for restructuring the whole nested fields situation?

@erquhart
Copy link
Contributor Author

@lmcorreia I've been intending a wholesale refactor that would help address this, but I'm not certain when that will happen. You're definitely welcome to take this on, and I'm happy to help if you run into any issues.

Sent with GitHawk

@lmcorreia
Copy link
Contributor

@erquhart Cool, I'll take a fresh look at the HOC mechanics for this and try to suggest a new approach in a PR asap

@barthc
Copy link
Contributor

barthc commented Jan 24, 2019

@erquhart this issue already have a PR.

@lmcorreia
Copy link
Contributor

lmcorreia commented Jan 24, 2019

@barthc #1873 - is this it?

@lmcorreia
Copy link
Contributor

@erquhart ^^ From the little I've dug into it, this seems to cover what can be done for the moment without getting too much into the foundations of the List and Object widgets so I'd take it as a closing point for this issue.

I'll still dig around a little more to see if I can come up with some contribution for the future refactoring but that's beyond the scope of the issue, I believe.

@talves
Copy link
Collaborator

talves commented Jan 24, 2019

@lmcorreia Can you check out the PR and run tests and see if it solves the issue? That would be of great help. Would also help determine if there is any change needed that improves and work with @barthc or to discuss.

@barthc
Copy link
Contributor

barthc commented Jan 24, 2019

@lmcorreia Yes, that is the PR, and as @talves suggested any improvement to the PR is welcomed. At the moment, I will be updating the PR to fix conflicts and test against the new types list widget.

@lmcorreia
Copy link
Contributor

@barthc I've reviewed and tested your code and it all looks good. I've made a few suggestions but nothing extraordinary. They don't really change the logic in your code but I tested them anyway, same result - all good from what I've tested, validation (presence and pattern) still works in "normal" fields and now also works in fields in lists and objects/objects inside lists. 👍

cc/ @talves

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

Successfully merging a pull request may close this issue.

6 participants