Content Model: Fix #1738 delete space issue by JiuqingSong · Pull Request #1740 · microsoft/roosterjs

JiuqingSong · 2023-04-19T23:01:20Z

This change addresses 3 issues:

For content such as "a b", and backspace after "b", we should not delete the space. This is done in deleteSelections.ts by changing the space after "a" to be   when delete. Similar for DELETE key case
For content such as "a[SPACE][SPACE]b", when put cursor right before "b", browser will leave the second space after cursor so Content Model need to recognize this space and ignore it since it isn't rendering anything. This is done by creating a new model API normalizeParagraph and call it from normalizeContentModel. There are bunch of cases of spaces need to be handled there, see comments in code for more info
For   at then end of a segment (not the last text segment), we need to replace it to be a normal space if the second last char is not space so that if there is another long segment after it (e.g. a hyperlink), browser can wrap before the link.

Related test cases added. And in this change I also enabled ContentModelEditPlugin in demo site by default.

BryanValverdeU · 2023-04-21T16:27:00Z

Issue seem to repro with this snapshot. But only because the first chars are wrapped in a element. Not sure if it is in scope of this PR.

<div><span style="font-family: &quot;Segoe UI&quot;, &quot;Segoe UI Web (West European)&quot;, &quot;Segoe UI&quot;, -apple-system, BlinkMacSystemFont, Roboto, &quot;Helvetica Neue&quot;, sans-serif; font-size: 10.5pt;"><b>a&nbsp; </b>b</span></div><!--{"type":0,"isDarkMode":true,"start":[0,0,1,1],"end":[0,0,1,1]}-->

JiuqingSong · 2023-04-21T21:17:57Z

Issue seem to repro with this snapshot. But only because the first chars are wrapped in a element. Not sure if it is in scope of this PR.

<div><span style="font-family: &quot;Segoe UI&quot;, &quot;Segoe UI Web (West European)&quot;, &quot;Segoe UI&quot;, -apple-system, BlinkMacSystemFont, Roboto, &quot;Helvetica Neue&quot;, sans-serif; font-size: 10.5pt;"><b>a&nbsp; </b>b</span></div><!--{"type":0,"isDarkMode":true,"start":[0,0,1,1],"end":[0,0,1,1]}-->

Good catch. I have made a fix

* add tests for image edit * fix build * Move getPasteSource from Plugins to Dom (#1708) * init * try fix tests * Content Model minor fix: Change parameter type (#1720) * Content Model: Fix #1702 Hyperlinking a text doesnt trigger ChangeSource.CreateLink event (#1703) * Content Model: Fix #1702 * fix build * fix test * fix build * Fix #1713 (#1718) * Fix Uncaught TypeError: Cannot read properties of undefined (reading 'parentNode') (#1725) * Fix Unhandled exception * fix test * remove unneeded changes * Decouple logic from Table Cell Selection into smaller files. (#1722) * Refactor * Add comment * Allow Content Model API to remove image border (#1733) * Modify API to allow accepting null * New demo button * Testing * Content Model: Fix #1684 (#1730) * Content Model: Improve edit plugin (#1728) * Content Model fidelity 1: Support lineHeight in list (#1731) * [1/3] Enable Strict mode in TableCellSelection Utils + Plugin (#1735) Enable Strict mode in Table Cell Selection Utils and the Plugin class * Content Model fidelity improvement 2: Support size in divider (#1732) * Content Model fidelity 2: Support size in divider * fix test * Content Model: Improve default format state (#1743) * Content Model: Improve default format state * add a test case * Add check for empty selectors, and empty CSS rule insertion (#1744) * add check for empty selectors, and empty cssRule * fix test case * Content Model: Fix #1738 delete space issue (#1740) * Content Model: Fix #1738 delete space issue * enable edit plugin in demo site by default * Fix space issue * fix cache behavior * Make additionalTagReplacements allow `null` value type. (#1745) * Fix additionalTagReplacements type in HtmlSanitizerOptions.ts * Make compiler happy * [2/3] Enable Strict mode on KeyDown Handler (#1736) * Table cell selection strict mode 1 * enable strict mode on handleKeyDown * [3/3] Enable Strict mode in Mouse Down Handler + Add Strict mode files (#1737) * Table cell selection strict mode 1 * enable strict mode on handleKeyDown * enable strict mode on mousedown * merge conflicts * Change from undefined to null * Revert "Paste as image feature" - convertPastedContentFromExcel (#1749) * revert only convertPastedContentFromExcel.ts * Leverage Content Model to insert the Pasted content from clipboard (#1716) In order to start using content model to insert clipboard content to the editor we need to do some refactor on some utilities: Move the createPasteFragment logic from roosterjs-editor-core package to roosterjs-editor-dom, so we can also use the logic in roosterjs-content-model. Add a new property to BeforePasteEvent to let other plugins modify the way we process elements in the paste fragment, this will be used later in another PR. Create a new core api createPasteModel that will use the PasteFragment to create and return a content model document. In the ContentModelEditor Class, override the paste function so, instead of using insertContent, use mergeModel function Bugs that may be fixes by leveraging Content Model with this change: #456 #1065 #1322 partially, only the paste part is fixed, copy needs to fixed in another PR. #920 * Don't return focus to editor when blur on image (#1742) * Don't return focus to editor when blur on image * Comment * Content Model fidelity improvement 3: Block format (#1751) * Content Model fidelity improvement 3: Block format * improve * fix test * Add a experimental feature for paste with ContentModel #1758 * 8.46.0 --------- Co-authored-by: Júlia Roldi <juliaroldi@microsoft.com> Co-authored-by: Bryan Valverde U <bvalverde@microsoft.com> Co-authored-by: Julia Roldi <87443959+juliaroldi@users.noreply.github.com> Co-authored-by: Ian Elizondo <ianeli@microsoft.com> Co-authored-by: Andres-CT98 <107568016+Andres-CT98@users.noreply.github.com> Co-authored-by: Ruijun Gao <rckgao@gmail.com>

* add tests for image edit * fix build * Move getPasteSource from Plugins to Dom (#1708) * init * try fix tests * Content Model minor fix: Change parameter type (#1720) * Content Model: Fix #1702 Hyperlinking a text doesnt trigger ChangeSource.CreateLink event (#1703) * Content Model: Fix #1702 * fix build * fix test * fix build * Fix #1713 (#1718) * Fix Uncaught TypeError: Cannot read properties of undefined (reading 'parentNode') (#1725) * Fix Unhandled exception * fix test * remove unneeded changes * Decouple logic from Table Cell Selection into smaller files. (#1722) * Refactor * Add comment * Allow Content Model API to remove image border (#1733) * Modify API to allow accepting null * New demo button * Testing * Content Model: Fix #1684 (#1730) * Content Model: Improve edit plugin (#1728) * Content Model fidelity 1: Support lineHeight in list (#1731) * [1/3] Enable Strict mode in TableCellSelection Utils + Plugin (#1735) Enable Strict mode in Table Cell Selection Utils and the Plugin class * Content Model fidelity improvement 2: Support size in divider (#1732) * Content Model fidelity 2: Support size in divider * fix test * Content Model: Improve default format state (#1743) * Content Model: Improve default format state * add a test case * Add check for empty selectors, and empty CSS rule insertion (#1744) * add check for empty selectors, and empty cssRule * fix test case * Content Model: Fix #1738 delete space issue (#1740) * Content Model: Fix #1738 delete space issue * enable edit plugin in demo site by default * Fix space issue * fix cache behavior * Make additionalTagReplacements allow `null` value type. (#1745) * Fix additionalTagReplacements type in HtmlSanitizerOptions.ts * Make compiler happy * [2/3] Enable Strict mode on KeyDown Handler (#1736) * Table cell selection strict mode 1 * enable strict mode on handleKeyDown * [3/3] Enable Strict mode in Mouse Down Handler + Add Strict mode files (#1737) * Table cell selection strict mode 1 * enable strict mode on handleKeyDown * enable strict mode on mousedown * merge conflicts * Change from undefined to null * Revert "Paste as image feature" - convertPastedContentFromExcel (#1749) * revert only convertPastedContentFromExcel.ts * Leverage Content Model to insert the Pasted content from clipboard (#1716) In order to start using content model to insert clipboard content to the editor we need to do some refactor on some utilities: Move the createPasteFragment logic from roosterjs-editor-core package to roosterjs-editor-dom, so we can also use the logic in roosterjs-content-model. Add a new property to BeforePasteEvent to let other plugins modify the way we process elements in the paste fragment, this will be used later in another PR. Create a new core api createPasteModel that will use the PasteFragment to create and return a content model document. In the ContentModelEditor Class, override the paste function so, instead of using insertContent, use mergeModel function Bugs that may be fixes by leveraging Content Model with this change: * Don't return focus to editor when blur on image (#1742) * Don't return focus to editor when blur on image * Comment * Content Model fidelity improvement 3: Block format (#1751) * Content Model fidelity improvement 3: Block format * improve * fix test * Add a experimental feature for paste with ContentModel #1758 * 8.46.0 --------- Co-authored-by: Júlia Roldi <juliaroldi@microsoft.com> Co-authored-by: Bryan Valverde U <bvalverde@microsoft.com> Co-authored-by: Julia Roldi <87443959+juliaroldi@users.noreply.github.com> Co-authored-by: Ian Elizondo <ianeli@microsoft.com> Co-authored-by: Andres-CT98 <107568016+Andres-CT98@users.noreply.github.com> Co-authored-by: Ruijun Gao <rckgao@gmail.com>

Content Model: Fix #1738 delete space issue

381e683

JiuqingSong requested review from Andres-CT98, BryanValverdeU, flyingbee2012, ianeli1 and juliaroldi April 19, 2023 23:01

JiuqingSong mentioned this pull request Apr 19, 2023

POC: Replace nbsp to space #1599

Closed

JiuqingSong added 2 commits April 19, 2023 16:11

Merge branch 'master' into u/jisong/1738

fc55957

enable edit plugin in demo site by default

a130945

BryanValverdeU approved these changes Apr 21, 2023

View reviewed changes

JiuqingSong added 2 commits April 21, 2023 11:01

Merge branch 'master' into u/jisong/1738

338d023

Fix space issue

ac05a8d

JiuqingSong added 2 commits April 21, 2023 15:14

fix cache behavior

ec23442

Merge branch 'master' into u/jisong/1738

c179f49

JiuqingSong merged commit eabe2f6 into master Apr 22, 2023

JiuqingSong deleted the u/jisong/1738 branch April 22, 2023 06:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Content Model: Fix #1738 delete space issue#1740

Content Model: Fix #1738 delete space issue#1740
JiuqingSong merged 7 commits into
masterfrom
u/jisong/1738

JiuqingSong commented Apr 19, 2023 •

edited

Loading

Uh oh!

BryanValverdeU commented Apr 21, 2023

Uh oh!

JiuqingSong commented Apr 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

JiuqingSong commented Apr 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BryanValverdeU commented Apr 21, 2023

Uh oh!

JiuqingSong commented Apr 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JiuqingSong commented Apr 19, 2023 •

edited

Loading