-
Notifications
You must be signed in to change notification settings - Fork 96
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
ENH Pass form validation result to client #1246
ENH Pass form validation result to client #1246
Conversation
7688245
to
966ee81
Compare
18f9e12
to
f0ab022
Compare
a6a2589
to
fbf881e
Compare
That's an interesting mechanism with the header, didn't know there was something like that. Always learning :) Looks ok to me, let's wait for the tests. |
Just came across this PR which I thought was worth noting here incase yo uhadn't seen it: silverstripe/silverstripe-elemental#921 |
fbf881e
to
f7403a7
Compare
dc9bc7b
to
ef580d8
Compare
ef580d8
to
a802828
Compare
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.
I'd tick "Approve" if I wasn't the original author on this PR
Approved and merged, but now I wonder whether it should've targeted 2 as an enhancement? But seeing the target branch was updated last week by @GuySartorelli I'm not sure. LMK. |
On its own, yes this is just an enhancement and belongs in 2 - but it was done specifically to work with silverstripe/silverstripe-elemental#930 in order to solve silverstripe/silverstripe-elemental#764 - so it's part of a wider work to fix a bug that's pretty egregious, and I think in this case warrants being released in CMS 4 as a patch. |
Issue silverstripe/silverstripe-elemental#764
Allows the ValidationResult to be passed to the client as part of a pjax response
Wrapping the JSON in a
<script type="application/json">
is something of a workaround due to the expectation that pjax returns HTML. If it's not wrapped in a<script type="application/json">
tag it will fail here https://github.com/silverstripe/silverstripe-admin/blob/1/client/src/legacy/LeftAndMain.js#L775