Sync with facebook/master #55
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Fix trailing comma lints I'm not sure why ESLint didn't catch these at some point on Github - but in any case we are fixing it now. I ran the `trailing-commas.js` js-codemod and it only touched one file for some reason. Probably I missed something about the jscodeshift API. In any case, I fixed the other lints that our internal linter was catching. * Use DraftEntity type instead of EntityMap type in helpers While supporting both the old and new APIs in Draft.js for Entity manipulation, we are calling to the old DraftEntity module under the hood instead of using the new system that would use an Immutable.js OrderedMap called the EntityMap. The DraftEntity module now has private methods for entity manipulation that don't warn, and the public API which we are deprecating has warnings and then calls the private methods. In short, there are places where the flow typing wasn't right because it was expecting EntityMap, and not DraftEntity. I don't know why flow didn't catch this when we merged facebookarchive#814 test plan: `flow src` * Fix flow error caused by passing command to RichUtils.handleKeyCommand The 'handleKeyCommand' takes either a 'DraftEditorCommand' type or any string. This allows users to have their own custom editor command types. I'm not sure how common this is, but our current flow typing allows it. The flow typing of 'RichUtils.handleKeyCommand' is more strict - it only allows 'DraftEditorCommand' types, and not custom strings. This causes an issue in one of our use cases, where we pass the 'command' received in 'handleKeyCommand' through to 'RichUtils.handleKeyCommand'. It seems like the 'RichUtils.handleKeyCommand' should have an API similar to 'DraftEditor.handleKeyCommand', and so we're loosening the flow type on 'RichTuils.handleKeyCommand'. Test Plan: `flow` * Fix flow error with DraftEditor.setClipboard This was causing flow errors in our internal code because we expect that 'null' or 'undefined' is ok as an argument to 'setClipboard'. Test Plan: `flow src` * Fix flow and lint errors in CharacterMetadata Again - I'm not sure why the github CI didn't catch these issues earlier. I wonder if the issue with the typing of Immutable.OrderedMap relates to the version of Immutable.js we are using or something.
…acebookarchive#858) Private methods should not be called as public methods. A recent PR (facebookarchive#814) added warnings to the public-but-deprecated API of DraftEntity, and then moved the logic for those methods into private methods that can be called within the Draft.js framework. These methods are "private" in the sense that we are only calling them within Draft. They should not be used outside of the Draft library, because they will be removed in an upcoming version of the library. We have a transform step that causes issues when an underscore-prefixed method is called outside of it's module, so changing the method names seems like the easiest way to deal with this issue and still send the message that they are private. There is a way to turn off the transformation step on a file-by-file basis but that would lead to touching more files than this approach, and this code will be deprecated in the next release anyway.
Simplifies code and minimizes new concepts.
Facebook has renamed these; just keeping things in sync.
…acebookarchive#872) * Update Readme.md Updated Readme.md to include a Getting Started section using information from the documentation. * Update Readme.md Adjusted some headings to be H3 to appear as sub-headings in the Getting Started section to provide better reading flow.
* EditorState.set() * inherited
Add DraftEditorDefaultProps type
* Implemented moveAtomicBlockBefore and moveAtomicBlockAfter in AtomicBlockUtils * Renamed EditorChangeType 'change-fragment' to 'move-block' and added to docs * Changed interface for moving atomic blocks: - Just use one function called moveAtomicBlock instead of three - Introduce DraftInsertionType for determining if a fragment shall replace another fragment, or if it shall be inserted before or after another fragment * Using ES6 variable declaration and getStart/EndKey instead of getAnchor/FocusKey
Added syntax highlight
…-patch-1 Improve README.md
…archive#667" (facebookarchive#871) This fixes some issues with the fix originally merged from PR facebookarchive#667[2] Credit to @balpert and @srmcconomy for this fix. For now I'm getting them back in sync. Also thanks to @davidchang for pinging me about landing this, @colinjeane for reporting this bug in facebookarchive#917 and submitting a nearly identical fix: facebookarchive#919 @colinjeane since we already had this fixed internally I'm going to use our approach to keep things in sync. fixes facebookarchive#917, facebookarchive#849 [2]: facebookarchive#667 We also had to tweak this compared to the current internal version to accomodate a recent change that makes 'editOnBeforeInput' and the other handlers take 'editor' as an explicit argument instead of assumming 'this' will be the editor; facebookarchive@e64c2c3 Note that we are updating the version of 'fbjs' required in order to get access to the 'setImmediate' polyfill.
In one case it seems the return type was just incorrect. We also clarified the comments for 'keyBindingFn' in order to avoid confusion - previously due to some Flow issues we had almost changed this flow type to be incorrect. See facebookarchive#894 Also fixes a hanging comma lint.
…acebookarchive#920) This fixes a minor flow error coming up in a FB use case, and makes the overall typing more consistent in this file.
Just another nudge to help folks with showing how a bug is reproduced.
* Update Draft.js docs to use 0.10.0 syntax and examples * Add 'API Migration' section to documentation and other minor edits
* Docs tweaks - Fix reference to older contentState - Change `href` to `url` for consistency with "link" example in case people copy-paste between both (which I did today) - Fix indentation in README * Update Advanced-Topics-Entities.md
Type `wwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwwww` in Chrome. Click at the beginning of the second line. Press space. It no longer deletes your text. That's a good thing. T17377963
Since the main logging method is stubbed out, this should have no effect on the open source Draft project. Adding this on Github in order to continue syncing the internal and open source version of Draft, to unblock releasing v0.10.1 of Draft. Context of this change: We have gotten reports of an error thrown from `setDraftEditorSelection` that completely breaks Draft functionality once it occurs. It happens very inconsistently, across multiple browsers, and we have not yet been able to find the root cause or reproduce it reliably. This logging may help us get more information about the problem.
* update use of createClass for React v15.5 * Update location of shallow renderer The 'shallow' renderer was moved in React v15.5, and updating this quiets a warning about it. **issue:** facebookarchive#1157
This should help a lot with "Failed to execute 'setStart' on 'Range'" exceptions we've logged in production and seen in bug reports. Test Plan: Type in editor, right-click in Chrome then "Translate to English", editing no longer breaks.
**what is the change?:** Draft uses some ES6 syntax and assumes users will pair it with an ES6 polyfill. See https://github.com/facebook/draft-js/blob/c12640f05e8328f5c97902c0c5e94a02ba9f58be/docs/Advanced-Topics-Issues-and-Pitfalls.md#polyfills **why make this change?:** When testing some examples in IE they were not rendering, and threw an error because we use `String.prototype.startsWith` in Draft. **test plan:** Open the 'convertFromHTML' and 'media' example files in IE, and they will render. **issue:** facebookarchive#1165
* 0.10.0 * Add missing link to CHANGELOG
**what is the change?:** Adding annotations for the upcoming v0.10.1 release. **why make this change?:** - saving time when actually doing the release - transparency **test plan:** Visual inspection **issue:** facebookarchive#1158
**what is the change?:** We had few things to note each week and were deferring updating the meeting notes. **why make this change?:** Based on [issue facebookarchive#1193](facebookarchive#1193) it sounds like the meeting notes will provide signal that the project is still maintained.
* Add mention of draft css in overview doc * Remove code example from Draft.css mention in overview doc
**what is the change?:** In order to enable auto-syncing between Github and www we need to get both to a point were they are in sync. This copies some misc. changes from the www version of Draft into the Github version of Draft. We are pulling in some changes as well. Includes changes from D5070771 Fix errors with single new in context (Note about above: TIL "by design you're not supposed to use new with Immutable.js data structures (well, except Records).") D5172412 [codemod] disable automock by default on as many remaining tests in www as possible @bypass-lint D5070207 [www][flowify] Start adding Flow typechecks to Style D4966517 [TF] Minor comments and nits for the DraftJs Composer D5109581 Add logging to catch Firefox Draft.js error **why make this change?:** To enable auto-syncing, to speed up speed of development. **test plan:** `yarn test` I'm pretty confident of these changes since they have all been present in FB Draft for some time. **issue:** facebookarchive#1244
…chive#1247) **what is the change?:** We either check for the 'getEditorKey' method on the 'renderedComponent._instance' or the 'renderedComponent._instance._instance' **why make this change?:** Internally we are using a different variation of the 'shallowRenderer' and this test needs to run internally at Facebook as well as on Github. This is not an ideal solution, but we can continue improving this test once cleaning up the inconsistencies between Facebook and Github. **test plan:** `jest src/component/base/__tests__/DraftEditor.react-test.js` **issue:** facebookarchive#1244
**what is the change?:** Adds missing pragma. **why make this change?:** This was causing a problem for users in some build set-ups. Fixes facebookarchive#982 Thanks @yuku-t for this solution. :) **test plan:** Ran `yarn build` and inspected `lib/Draft.js.flow` before and after this fix. It has the `@flow` pragma after, and did not before. **issue:** facebookarchive#982
) **what is the change?:** Based on issues from the community and internal experiments, we know that there are severe bugs affecting some mobile web uses of Draft.js. In particular, Android with certain keyboards and IME/rtl on Android. **why make this change?:** This information is important for folks when deciding whether Draft.js is the best choice for their use case. It may also get more interest in improving Draft.js for mobile web use cases. **test plan:** Opened the docs and visually inspected. (Flarnie will insert a screenshot.) **issue:** Related to facebookarchive#1224
| @@ -88,8 +89,41 @@ function editOnInput(editor: DraftEditor): void { | |||
| var domSelection = global.getSelection(); | |||
|
|
|||
| var {anchorNode} = domSelection; | |||
| if (anchorNode.nodeType !== Node.TEXT_NODE) { | |||
| return; | |||
| const isNotTextNode = | |||
There was a problem hiding this comment.
Do we understand enough about this section to confidently merge? (For now, flagging this for further personal investigation)
There was a problem hiding this comment.
I had to look at this specifically while merging. It is coming from facebookarchive#1155
There was a problem hiding this comment.
This line looks like it came from sophiebits@3b91e36, which actually appears to set draft_killswitch_allow_nontextnodes to false, which changes the functionality and might not be fully tested yet
There was a problem hiding this comment.
P.S. this entire naming scheme is incredibly confusing, because draft_killswitch_allow_nontextnodes is actually saying to not allow non text nodes...
There was a problem hiding this comment.
Sorry, wrong issue: facebookarchive#1104
There was a problem hiding this comment.
To that end, our version of DraftFeatureFlags is the same as Facebook's. This is the code running on Facebook itself.
There was a problem hiding this comment.
Totally - it's just something we should keep in mind that changed if we run into problems after shipping this
There was a problem hiding this comment.
Absolutely, this definitely needs to be well tested before we deploy.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This syncs the
releasebranch with the latest changes infacebook/master. Facebook has recently enabled syncing between their internal copy of Draft and the external version on GitHub and so is getting ready to accept a number of longstanding PRs. There are already quite a few changes and I'd rather pull in the current diff so that we don't continue to drift further out of sync.This is something we should be doing regularly in order to avoid massing drops like this one.