-
-
Notifications
You must be signed in to change notification settings - Fork 570
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
Displays validation error messages on control panel forms #5950
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for volto canceled.
|
✅ Deploy Preview for plone-components canceled.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you resolve the merge conflict?
invariantErrors = errorsList | ||
.filter((errorItem) => !('field' in errorItem)) | ||
.map((errorItem) => errorItem['message']); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to move some of this to a helper function that can be used here and in other components instead of duplicating the logic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
d465ffc
to
5cca767
Compare
@ichim-david I ran into the error in the "As editor, I can unlock a locked page" test again. I remember you found a solution to the problem: Do you intend to make this fixe? |
@davisagli done. |
@wesleybl yup I do ... time to dust up the old pull request I didn't add, April was not a good month for Volto work unlike March :) |
@wesleybl only left it there for 1 month .. if you would like to test it locally the locking test here is the pull request where I made the changes necessary to avoid errors after running it 20 times in a row as I've given you the comment on how todo it |
@davisagli @sneridagh can you take a look please? |
@wesleybl what's left of this one? |
63a7238
to
0e627af
Compare
@sneridagh I think it's ready to merge. There was a conflict but I resolved it. |
Fixes: #5274
Now the error is displayed:
To works properly, it needs: plone/plone.restapi#1771
But it can be merged separately without any problems.