This repository has been archived by the owner on Feb 6, 2023. It is now read-only.
Fixed bugs with deletion of entities between blocks and the atomic block deletion test case #1108
+38
−17
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
ouchxp
pushed a commit
to ouchxp/draft-js
that referenced
this pull request
Apr 7, 2017
jackyho112
added a commit
to jackyho112/draft-js
that referenced
this pull request
Apr 17, 2017
* get rid of the two boolean flags added in facebookarchive#1108 * rearrange code in a procedural/imperative style to be more understandable this fixes facebookarchive#1116
flarnie
pushed a commit
that referenced
this pull request
Jul 28, 2017
* Update copyright year to 2017 in footer on website (#1019) * Add meeting notes from 02-17-17 (#1021) * Add meeting notes from 02/24/17 (#1025) * Add CNAME * Updates to move the site to draftjs.org * Fixes to website builder scripts - Include extensionless files in build - Update convert.js so it handles Windows newlines * Fix links in docs sidebar * Add .gitkeep to /website/src/lib We need this directory to be present for the initial build of the website. * Only include files in glob, not directory. References #1029 * Fix race condition in website builder script There was a race condition in the website generation script. It calls convert.js to convert Markdown to JavaScript, and then immediately looks for the resulting js files. However, convert.js uses the async version of `glob()`. This means that the code in generate.js that looks for these files can execute **before** the conversion in convert.js finishes, resulting in no pages being built. When this happens on TravisCI, it deletes all the pages on the site, due to the `rm -Rf *` in `publish.sh`. I've simply switched convert.js to use the synchronous version of `glob()` so that it's guaranteed to finish before the generation happens. * Fix editor losing focus after first word on Android. (#907) * Fix editor losing focus after first word on Android. This patch narrows the scope of `restoreEditorDOM` to only blow away the `DraftEditorContents` instead of the entire editor. The key difference is that the `contentEditable` DOM element is not replaced, which means that the soft keyboard isn't dismissed on Android. Manually tested against "Cut" and "IME" tests: https://github.com/facebook/draft-js/wiki/Manual-Testing#cutcopypaste https://github.com/facebook/draft-js/wiki/Manual-Testing#ime Fixes #888, Ref #626, Ref #102 * Fix editor losing focus after first word on Android. This patch narrows the scope of `restoreEditorDOM` to only blow away the `DraftEditorContents` instead of the entire editor. The key difference is that the `contentEditable` DOM element is not replaced, which means that the soft keyboard isn't dismissed on Android. Manually tested against "Cut" and "IME" tests: https://github.com/facebook/draft-js/wiki/Manual-Testing#cutcopypaste https://github.com/facebook/draft-js/wiki/Manual-Testing#ime Fixes #888, Ref #626, Ref #102 * Small update keybinding docs. (#1023) * Bind handleKeyCommand in the constructor. It's bad practice to bind functions in render. Updated it to handle binding in the constructor. so it is only bound once. * Update Advanced-Topics-Key-Bindings.md * Add weekly meeting notes for 3-10-2017 (#1064) * Change website main font-weight to normal. (#1069) * Add `editorKey` prop for SSR (#796) * add editorKey functionnality * add simple tests for editorKey functionality * fix flow error * minimal working error case for universal rendering adds a small express-backed universal react app that only accepts `/` requests but that exhibits a client/server mismatch issue * fix failing test when length of generated hash is small * Update universal example since this PR was created some changes were made and this updates the example to fit with our other examples. - moves into the version specific subdirectories - updates package.json - removes some ES6 destucturing temporarily because polyfill seems to be not loading correctly * Correctly delete immutable and segmented entity content when at the edge of a selection. Fixes #1063. (#1065) * add documentation for `editorKey` prop (#1087) * better description for polyfilling Draft in quickstart installation docs just one of those things that i didn't pay much mind to until i was trying to isolate a bug i'm having in ie that i'm testing using a c-r-a sandbox. * fixed git repo url in docs/sever readme * add more detailed comments around editorKey prop * add reference docs for editorKey prop in `<Editor />` * Revert "Correctly delete immutable and segmented entity content when at the edge of a selection" (#1095) * Revert "add documentation for `editorKey` prop (#1087)" This reverts commit c12640f. * Revert "Correctly delete immutable and segmented entity content when at the edge of a selection. Fixes #1063. (#1065)" This reverts commit 413e63a. * Initial fix for text insertion bugs in Android 5.1 (#1033) Related to issue #1011 There is a bug in Android 5.1 (not 5.0 oddly enough) that makes text entry nearly unusable. It's documented in detail here - #1011 Logging everything shows that; - when we have a block with text and spaces, and start deleting the characters one by one, there is an 'onInput' for each deletion that updates the text. - when you delete the very last character, the 'anchorNode' of the domSelection has a type that is not a text node. So we skip updating at all, and that means the last character doesn't actually get properly deleted from editorState. - Then the next time something fully updates the editorState, that last character shows up again. The root issue is that Android 5.1, when we tested it, was in "composition" mode and the current handlers are not really built to handle text deletion properly in composition mode. We're looking forward to finding a more complete solution to the root cause - for now this change improves the usability on mobile and doesn't break anything. * Add Feature Flags in Draft, and add Entity Deletion change under GK (#1104) We use feature flags in the FB version of Draft to test new and potentially unstable changes. This mirrors out the flags we currently have and sets the stage for keeping things in sync going forward. Now internal use of feature flags will not block syncing and releasing Draft. This is part of an effort to make releases simpler and more frequent, and keep the internal and external versions of Draft in sync. As part of matching up the internal and external Draft flags we added the changes from PR #1065 by @mmmoussa, with the main change under a flag that is set to false. ---Test Plan Manually tested the examples in Chrome, and we will do the full test plan before releasing v0.10.1. * Fix missing commas (#1107) Updated comma-dangle eslint rule and fixed resulting errors * Expose props for disabling auto-correct, auto-complete, auto-capitalize (#1035) Wanted: manual testing of this PR. There is some really weird behavior in Android 5.0 when auto-correct happens. More details here; #1010 Setting 'spellCheck' to 'false' didn't seem to be enough to stop this. My quick fix for now is to expose more properties on Draft for controlling autocorrect. This could also be useful to people in general, since there are often complaints about autocorrect on mobile web. Long term we will be addressing the root causes of the bugs with mobile web, but this seems like a useful change regardless. We did some testing with mobile web using these props. Ideally I'd like to get testing done of each prop in each supported browser, and document the effects. If anyone can help verify different behaviors please comment here, otherwise I'll get to it soon. Note that we'll update the documentation once this change is actually included in a released version of Draft.js. * Add text directionality override prop to DraftEditor (#1034) * Add text directionality override prop to DraftEditor Credit for this change goes to @mmmoussa - Added an optional textDirectionality prop of type DraftTextDirectionality to the DraftEditor component which allows overriding the directionality of all blocks in the editor. - Fixed lint errors in the modified Draft.js files. Note that we will be updating the DraftTextDirectionality type definition to move it to the `fbjs` shared utils, possibly along with related text directionality utilities. * Hold off on updating docs with textDirectionality Since we haven't released this new prop yet let's not add it to the documentation until we do. * Use BidiDirection instead of own directionality type Thanks to @zpao for pointing out that fbjs has BidiDirction. * Add bundles from 'universal' example to eslintignore (#1109) * Pass fresh editorState to edit handlers (#1112) There have been bugs reported in cases where the users of Draft are updating 'editorState' in their custom handlers for keyDown, paste, etc. This can be a problem when they are using an 'editorState' that is in the parent's props or state and is not in sync, at that moment, with the editorState in Draft. This allows the parent to use the most updated editorState in custom handlers. This change was already reviewed by @spicyj and credit for this fix goes to @AlaNouri for making this change. * [Part 2] Pass fresh editorState to edit handlers (#1113) I had mirrored parts of this change from FB to github but missed some things. This is the second part of the same change - context below. Earlier part merged as #1112 --- There have been bugs reported in cases where the users of Draft are updating 'editorState' in their custom handlers for keyDown, paste, etc. This can be a problem when they are using an 'editorState' that is in the parent's props or state and is not in sync, at that moment, with the editorState in Draft. This allows the parent to use the most updated editorState in custom handlers. This change was already reviewed by @spicyj and credit for this fix goes to @AlaNouri for making this change. * Fixed bugs with deletion between blocks and atomic block deletion test case (#1108) * Configure dist output to use UMD format (#1090) * Update Entities docs in Advanced Topics section to correct an error (#1141) * Update Advanced-Topics-Entities.md * Update APIReference-ContentState.md (#1150) * Fixed incorrect entity documentation (#1149) * Update lint rules, flow version, and inline documentation (#1126) * Docs tweaks (#1139) * 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 * Fix mystery text deletion (#1155) 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 * Add logging for hard-to-reproduce bug (#1156) 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. * Fix errors from react update (#1159) * 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:** #1157 * Mark editor element as notranslate (#1162) 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. * Fix "universal" demo on Windows. (#1168) Closes #1166 * Add es6 polyfill to all examples (#1171) **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:** #1165 * More logging for selection failures (#1175) * 0.10.0 (#1177) * Add changelog link (#1178) * 0.10.0 * Add missing link to CHANGELOG * Removed unnecessary invariant usage (#1181) * Add 'Unreleased' version of v0.10.1 change log (#1183) **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:** #1158 * 0.10.1 * Update weekly meeting notes from March-April 2017 (#1195) **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 #1193](#1193) it sounds like the meeting notes will provide signal that the project is still maintained. * Add documentation about textDirectionality prop (#1037) * Add mention of draft css in overview doc (#1208) * Add mention of draft css in overview doc * Remove code example from Draft.css mention in overview doc * Add meeting notes from last two weeks (#1229) * Add weekly meeting notes (#1235) * Copy over misc. changes from Facebook (#1245) **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:** #1244 * Tweak syntax of accessing '_instance' in DraftEditor test (#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:** #1244 * Fix line breaking in docs * move all .gitignores into toplevel (#1218) * Add flow pragma to 'Draft.js' (#1262) **what is the change?:** Adds missing pragma. **why make this change?:** This was causing a problem for users in some build set-ups. Fixes #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:** #982 * Add docs about lack of support for mobile browsers (#1267) **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 #1224 * Update APIReference-Editor.md (#1179) * Update APIReference-Editor.md update API reference Editor to include onFocus and onBlur props * Update APIReference-Editor.md * Move DraftFeatureFlags to stubs (#1278) **what is the change?:** Moved 'DraftFeatureFlags' to 'stubs' directory, and make it export 'DraftFeatureFlags-core'. **why make this change?:** The 'DraftFeatureFlags' file is, *I think*, the last file which is out of sync with our internal FB version of Draft. That is because we actually use different flag configs within FB than the defaults for OS. **test plan:** `yarn install && yarn build && yarn test` and play with example files **issue:** issue #1244 * [Draft] Fix first-character prevention with async rendering Summary: When typing the first character of a block, Draft always tries to call .preventDefault() on the beforeinput(/keypress) even for that character's insertion, because browsers are likely to mess up the DOM into a form we don't expect. We check this by reading from the editorState and checking whether the selection is at the start of a block (either at the beginning of a line that is present, or if there is no text in a line yet). In the case of React Fiber's async rendering, it was possible to get into a state where when inserting two characters AB, A's insertion would be correctly prevented and queued to occur via React rendering, but B's insertion would not get prevented, so it would happen natively even though in many cases A's rendering had not occurred yet. We already had two copies of editorState we could consider to determine which path to take: * ._latestEditorState is the last state we have enqueued a rerender for. We always read from this in most of our event handlers so that if you type multiple characters, they are queued in order and don't clobber each other, regardless of whether there has been a rerender in the meantime. * .props.editorState is the last state that we've run cWRP/sCU/cWU/render on. In sync React, this always matches what we have actually rendered to the DOM (by the time we make it to an event handler). In async React, it doesn't, since we don't always run render in the same tick that we commit. Checking isSelectionAtLeafStart on .props.editorState is more correct since it more closely resembles the current state of the DOM (and this logic should depend on the actual state of the DOM, not the model state), but it is still incorrect in some cases. I added: * ._latestCommittedEditorState is the last state that we've committed to the DOM and run cDU on. In sync React, this matches .props.editorState. In async React, it is meaningfully different and does not get updated until the commit, while .props.editorState could get updated during a render that gets paused or aborted. Changing the logic to call isSelectionAtLeafStart on ._latestCommittedEditorState makes it so we correctly determine when to prevent native character insertion. (I think.) Test Plan: Open plain text editor with React's useSyncScheduling set to false. With the editor empty, press two characters on your keyboard simultaneously. Previously, this would cause an error dialog (.removeChild of the <br> node within DraftEditorLeaf throws since it's already deleted); now it seems to reliably inserts both characters. I did this 40 or so times in a row without it crashing. (If changed to .props.editorState but not ._latestCommittedEditorState, it fails about once every dozen or two times on my machine. Could easily be different on another machine due to scheduling randomness.) I still occasionally saw an issue where you end up with both a <span data-text="true"> and a <br data-text="true"> rendered. I believe this is due to other bugs in async React, not any failing of Draft. * [draft] Fix typing into text nodes containing Tab Summary: lolwaffle chrome is a mystery. if you have like ``` <span>ABC\tDEF</span> ``` (where \t is a tab), typing *anywhere* in that span will break the span into two pieces leaving you with like ``` <span>AB</span>x<span>C\tDEF</span> ``` Test Plan: paste "a\tbc" into text field. type into it. no crashes. * Fix-up diff for enabling 'ship-it' and 'import-it' (#1286) **what is the change?:** what is the change?: Development on Draft.js is causing Github and our internal version to get out of sync. This pulls various changes from our internal version into the Github project, so that they will be basically the same. Some of the included changes; Reorder requires via a codemod, maintained by an ESLint rule (D5117037) Fix 'first character prevention' bug that is triggered by Fiber async rendering (D5161053) Add 'z-index' to placeholder (D4769133) Add possible fix for issue #1020 (D5261172) Add 'flowfixme' comment (D5337485) Fix 'flowfixme' comment (D5389367) For reference, this patch was generated by the commands listed in D5273910. why make this change?: We want to keep them in sync going forward, and will be enabling scripts to automate mirroring changes back and forth. test plan: yarn install && yarn build && yarn test and manually tested examples. issue: related to #1244 fbshipit-source-id:a5d95c34f709c6fabcf0ceec876d4edb93881f85 * Empty commit to properly add the fbshipit-source-id See #1244 In my previous commit I had the wrong fbshipit-source-id. fbshipit-source-id: f8f26ac6b6d22739cb0d1c73ae8fa613acf26758 * Add trailing comma Summary: This makes OSS lint fail. Created from Diffusion's 'Open in Editor' feature. Reviewed By: flarnie Differential Revision: D5413446 Tags: accept2ship fbshipit-source-id: a68974062eab16cd834784a5493ce781bb4b7798 * Add weekly meeting notes from July 7th and 14th Summary: Closes #1290 Differential Revision: D5432441 fbshipit-source-id: 1709a5ee540164f5859382adcec7c19d97e59b46 * Update fbjs and fbjs-scripts to pull in bug fix Summary: **what is the change?:** Updated the versions of `fbjs` and `fbjs-scripts` **why make this change?:** Latest versions include a bug fix[1] that fixes #914 [1]: facebook/fbjs#244 **test plan:** 0) Use `n` to set my local version of node.js to `7.6.0`, with npm v4.1.2 1) Ran `yarn upgrade && yarn build` and saw no errors **issue:** #914 Closes #1291 Differential Revision: D5432468 fbshipit-source-id: 2dda1e9efeef662a807c87abb0bab12340c6289f * using newer 'ref' syntax Summary: *Before* submitting a pull request, please make sure the following is done... 1. Fork the repo and create your branch from `master`. 2. If you've added code that should be tested, add tests! 3. If you've changed APIs, update the documentation. 4. Ensure that: * The test suite passes (`npm test`) * Your code lints (`npm run lint`) and passes Flow (`npm run flow`) * You have followed the [testing guidelines](https://github.com/facebook/draft-js/wiki/Testing-for-Pull-Requests) 5. If you haven't already, complete the [CLA](https://code.facebook.com/cla). Please use the simple form below as a guideline for describing your pull request. Thanks for contributing to Draft.js! - **Summary** [...] **Test Plan** [...] Closes #925 Differential Revision: D5442872 fbshipit-source-id: a99321f173a3cb64fe01b83360dbe7f30cbe8adc * Fix minor typo in docs Summary: Noticed a missing character in "redering" on "Custom Block Rendering" docs page. Closes #1239 Differential Revision: D5433774 fbshipit-source-id: dae4939ab68b35422921f05440f6e6efd4e2e51a * Removing unnecessary space Summary: Removing unnecessary space after from Entities's Introduction Closes #1207 Differential Revision: D5428786 fbshipit-source-id: 13531f192b911b6f1e8c36ee1edcd15d5a32248b * Add docs on the lack of support for mobile web Summary: **what is the change?:** Improvind docs by mentioning the lack of mobile web support **why make this change?:** For certain Android keyboards, and with IME in particular, Draft.js has some known issues. We should document this so that folks are aware. **test plan:** Ran the website and manually tested.  **issue:** #1224 Closes #1228 Differential Revision: D5462589 fbshipit-source-id: 9354e7d9a73622927da69acded4db5b06b1a1c6d * Add aria-multiline attr to DraftEditor Summary: `aria-multiline` is used to indicate to assistive technology agents that a textbox is a multiline textbox. When `aria-multiline` is true, an AT should treat return key presses as carriage returns, not submit actions. https://www.w3.org/TR/wai-aria-1.1/#aria-multiline teeeeeeeested: `npm run test && npm run lint && npm run flow` Closes #1295 Differential Revision: D5464455 fbshipit-source-id: e0033db62f4b0529875ee77fe5b0635222b16af4 * Clarify comment above `tryToRemoveBlockStyle` Summary: I found the comment above `RichUtils.tryToRemoveBlockStyle` a bit confusing while trying to understand the function's logic. I would argue that the current comment's inclusion of information about where it is used is unnecessary, and makes it seem as if its interaction with those key commands was implemented inside the function itself. Closes #1135 Differential Revision: D5477479 fbshipit-source-id: a8cee503bf5459c63ac58504f5c112a1a249b64a * Corrected documentation typo, example code throws an error. Summary: **Summary** `this.props` throws an exception. Because we don't have component state available at that point. It's pretty obvious but still, it can be a little confusing. **Test Plan** - n/a Closes #1238 Differential Revision: D5477594 fbshipit-source-id: ec2169666d3134595acbcb69902fd2ea4b6ae3b0 * Fix paths * Fix linting errors
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Summary
Fixed bugs with entity deletion when the start and end of the selection are not in the same block. Also fixed a bug with the atomic block deletion test case which was identified when PR #1094 was opened.
Test Plan
Ensured all unit tests pass with the draft_segmented_entities_behavior feature flag enabled and disabled. Also tested all deletion edge cases related to mutable, segmented, and immutable entities, along with atomic block deletion.