-
Notifications
You must be signed in to change notification settings - Fork 9k
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
Fix formatting Validation Error for Map items #8333
Fix formatting Validation Error for Map items #8333
Conversation
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.
Thanks for the fix, I can confirm that it works.
Please take a look at my comments.
errors.forEach( e => result.push(e)) | ||
errors | ||
.map( e => { | ||
if (e instanceof Map) { |
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 change this to if (Map.isMap(e))
, to use a method from Immutable
?
errors | ||
.map( e => { | ||
if (e instanceof Map) { | ||
return `${e.get("propKey")} : ${e.get("error")}` |
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.
Can you remove the spacing before the colon ${e.get("propKey")}: ${e.get("error")}
?
return e | ||
} | ||
}) | ||
.forEach( e => result.push(e)) |
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.
Can we go for a little bit more concise way of writing this code?
errors
.map((e) => (Map.isMap(e) ? `${e.get("propKey")} : ${e.get("error")}` : e))
.forEach((e) => result.push(e))
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.
Looks good, but we require either a unit test or e2e test to verify the code change. Would you be able to provide one?
Refactored the code and added an e2e test in #9687. Thanks for this again, your contribution will be properly attributed. |
Description
In the validationError Array rendered there are string and maps.
The maps Item are not rendered correctly as you cam see in the image below
I've fixed the code to render properly these items
Checklist
My PR contains...
src/
is unmodified: changes to documentation, CI, metadata, etc.)package.json
)My changes...
Documentation
Automated tests