-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Add default value for blockRenderMap #1302
Add default value for blockRenderMap #1302
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 taking the time to submit this PR!
Can you add a "Test Plan" to make sure that Draft works as expected when you only specify blockRenderFn
and when you specify both blockRenderFn
and blockRenderMap
?
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.
This looks good - could you just add a quick unit test or describe how to manually test it? Thanks! 🌟
Would still like to get this fix in - I'll add a quick test based on the example from the issue. |
**what is the change?:** Adding a test for the solution to issue facebookarchive#1211 In general I prefer testing the public API, so we actually render an example editor and trigger the situation which used to throw an error. **why make this change?:** We need better unit tests for Draft, and this will help stop the bug from recurring if someone randomly removes that check in the future. **test plan:** `yarn run test src/component/contents/__tests__/DraftEditorContent.react-test.js` We tried temporarily undoing the fix and running the test, and verified that it failed. **issue:** facebookarchive#1211
Adding this to the queue for merging~✨ |
@flarnie has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Summary: **Summary** PR for issue 1211 facebookarchive/draft-js#1211 **Test Plan** <s>no need</s> **Edit by Flarnie:** added unit test Closes facebookarchive/draft-js#1302 Reviewed By: zpao Differential Revision: D5881334 Pulled By: zpao fbshipit-source-id: b8d57b23a1654fe9cdcf84b74b25617413ef6248
Summary: **Summary** PR for issue 1211 facebookarchive/draft-js#1211 **Test Plan** <s>no need</s> **Edit by Flarnie:** added unit test Closes facebookarchive/draft-js#1302 Reviewed By: zpao Differential Revision: D5881334 Pulled By: zpao fbshipit-source-id: b8d57b23a1654fe9cdcf84b74b25617413ef6248
Summary: **Summary** PR for issue 1211 facebookarchive/draft-js#1211 **Test Plan** <s>no need</s> **Edit by Flarnie:** added unit test Closes facebookarchive/draft-js#1302 Reviewed By: zpao Differential Revision: D5881334 Pulled By: zpao fbshipit-source-id: b8d57b23a1654fe9cdcf84b74b25617413ef6248
Summary
PR for issue 1211
#1211
Test Plan
no needEdit by Flarnie: added unit test