Skip to content
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.

Sign up for GitHub

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

Jump to bottom

MergeModel, do not inherit the styles of table when splitting the param #2016

Merged
merged 5 commits into from
Aug 9, 2023
Merged

MergeModel, do not inherit the styles of table when splitting the param #2016

merged 5 commits into from
Aug 9, 2023

Conversation

BryanValverdeU
Copy link
Contributor

@BryanValverdeU BryanValverdeU commented Aug 9, 2023 •
edited
Loading

#2015
We should not add the table format when splitting the paragraph, as this can lead to border styles to be incorrectly applied to the splitted pagraph

Before
image

After
image

JiuqingSong
JiuqingSong reviewed Aug 9, 2023
View reviewed changes
packages-content-model/roosterjs-content-model-editor/lib/modelApi/common/mergeModel.ts Outdated
@@ -287,7 +287,8 @@ function splitParagraph(markerPosition: InsertPoint, newParaFormat: ContentModel

function insertBlock(markerPosition: InsertPoint, block: ContentModelBlock) {
const { path } = markerPosition;
const newPara = splitParagraph(markerPosition, block.format);
const newParaFormat = block.blockType === 'Table' ? {} : block.format;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel we should get this format only when block is paragraph. For all other block types, such as ListItem, Table, Divider, Entity, we should not inherit its format. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you. I was not too sure to handle those block types too in this PR. But now that you mention it I will add that change too.

@BryanValverdeU BryanValverdeU merged commit 72f0c87 into master Aug 9, 2023
7 checks passed
Andres-CT98 added a commit that referenced this pull request Aug 18, 2023
@Andres-CT98 @JiuqingSong
@BryanValverdeU @juliaroldi
Version bump 8.54.0 (#2042)
e2ae751 
* Content Model: Improve cache behavior (#1999)

* Content Model: Improve cache behavior

* fix build

* fix comment

* Skip Trigger plugin event on plain text paste (#2011)

* init

* remove unneeded change

* Update return type

* Fix 218869: Do not allow dragging on readonly content (#2010)

* Fix 218869: Do not allow dragging readonly content

* fix test

* Content Model: Fix 194024 and 220289 (#2012)

* Content Model: Fix 221290 Support float for image (#2013)

* Content Model: improve formatWithContentModel 1 (#2001)

* Content Model: Improve cache behavior

* fix build

* Content Model: improve formatWithContentModel

* Content Model: improve formatWithContentModel 2 (#2002)

* Content Model: Improve cache behavior

* fix build

* Content Model: improve formatWithContentModel

* Content Model: improve formatWithContentModel 2

* fix format

* WIP

* fix handles

* MergeModel, do not inherit the styles of table when splitting the param (#2016)

* init

* init

* address comment

* update test names

* fixes

* Demo site: Fix insert link button in Content Model ribbon (#2018)

* Content Model: insertEntity API (#1800)

* Content Model insertEntity

* improve

* improve

* Content Model: Improve cache behavior

* fix build

* Content Model: improve formatWithContentModel

* Content Model: improve formatWithContentModel 2

* Improve

* fix build

* fix build

* improve

* add test

* add test

* add test

* add test

* fix dark color

* fix test

* fix build and test

* crop

* fix xase

* check cell exist

* Fix #1752, rename header to heading (#2020)

* fix cell empty cells

* fix flipped image

* Update logic to decide if we need to merge a table on paste. (#2022)

* Fix TableSelectionCopy

* update unit tests

* Fix 2

* address comment

* Content Model: Rename a test file (#2029)

* Fix Triple clicking a single cell selecting more than one (#2024)

* fix triple click, optimisation

* Remove `display: flex` style on paste (#2031)

* init

* Fix

* Replace first cell content if input while on cell selection (#2030)

* select first cell content and empty, add undo if change

* Content Model: Fix 222135 (#2035)

* Fix 222135

* fix build

* Content Model: Fix 219312 (#2036)

* Fix regression when creating the BeforePasteEvent (#2039)

* init

* fix build

* Content Model: Fix 220050 (#2037)

* Content Model: Fix 220050

* Fix build

* improve

* improve

* Simplify the domToModel call in `paste.ts` (#2040)

* add more changes

* fix build

* fix test

* Content Model: Support vertical-align for image (#2041)

* Content Model: Support vertical-align for image

* fix build and test

---------

Co-authored-by: Bryan Valverde U <bvalverde@microsoft.com>

* version bump

---------

Co-authored-by: Jiuqing Song <jisong@microsoft.com>
Co-authored-by: Bryan Valverde U <bvalverde@microsoft.com>
Co-authored-by: Júlia Roldi <juliaroldi@microsoft.com>
Co-authored-by: Julia Roldi <87443959+juliaroldi@users.noreply.github.com>
@JiuqingSong JiuqingSong deleted the u/bvalverde/mergeModel branch September 8, 2023 22:35
ianeli1 added a commit that referenced this pull request Sep 11, 2023
@ianeli1 @JiuqingSong
@BryanValverdeU @juliaroldi @Andres-CT98
Bump rooster to 8.55.0, content model to 0.15.0 (#2060)
54e3f91 
* Content Model: Improve cache behavior (#1999)

* Content Model: Improve cache behavior

* fix build

* fix comment

* Skip Trigger plugin event on plain text paste (#2011)

* init

* remove unneeded change

* Update return type

* Fix 218869: Do not allow dragging on readonly content (#2010)

* Fix 218869: Do not allow dragging readonly content

* fix test

* Content Model: Fix 194024 and 220289 (#2012)

* Content Model: Fix 221290 Support float for image (#2013)

* Content Model: improve formatWithContentModel 1 (#2001)

* Content Model: Improve cache behavior

* fix build

* Content Model: improve formatWithContentModel

* Content Model: improve formatWithContentModel 2 (#2002)

* Content Model: Improve cache behavior

* fix build

* Content Model: improve formatWithContentModel

* Content Model: improve formatWithContentModel 2

* fix format

* WIP

* fix handles

* MergeModel, do not inherit the styles of table when splitting the param (#2016)

* init

* init

* address comment

* update test names

* fixes

* Demo site: Fix insert link button in Content Model ribbon (#2018)

* Content Model: insertEntity API (#1800)

* Content Model insertEntity

* improve

* improve

* Content Model: Improve cache behavior

* fix build

* Content Model: improve formatWithContentModel

* Content Model: improve formatWithContentModel 2

* Improve

* fix build

* fix build

* improve

* add test

* add test

* add test

* add test

* fix dark color

* fix test

* fix build and test

* crop

* fix xase

* check cell exist

* Fix #1752, rename header to heading (#2020)

* fix cell empty cells

* fix flipped image

* Update logic to decide if we need to merge a table on paste. (#2022)

* Fix TableSelectionCopy

* update unit tests

* Fix 2

* address comment

* Content Model: Rename a test file (#2029)

* Fix Triple clicking a single cell selecting more than one (#2024)

* fix triple click, optimisation

* Remove `display: flex` style on paste (#2031)

* init

* Fix

* Replace first cell content if input while on cell selection (#2030)

* select first cell content and empty, add undo if change

* Content Model: Fix 222135 (#2035)

* Fix 222135

* fix build

* Content Model: Fix 219312 (#2036)

* Fix regression when creating the BeforePasteEvent (#2039)

* init

* fix build

* Content Model: Fix 220050 (#2037)

* Content Model: Fix 220050

* Fix build

* improve

* improve

* Simplify the domToModel call in `paste.ts` (#2040)

* add more changes

* fix build

* fix test

* Content Model: Support vertical-align for image (#2041)

* Content Model: Support vertical-align for image

* fix build and test

---------

Co-authored-by: Bryan Valverde U <bvalverde@microsoft.com>

* Remove deprecated colors from borders, text and background. (#2045)

* Support more border styles

* init

* Revert  unrelated change

* Fix dependencies

* address comments

* try fix build

* reselection

* add space

* Content Model: Improve insertEntity (#2047)

* Content Model: Improve insertEntity

* fix test

* Content Model: Fix selection of entity (#2051)

* Content Model: Always return size in pt when getFormatState (#2052)

* Content Model: trigger ShadowEdit events (#2053)

* Content Model: trigger ShadowEdit events

* improve

* Content Model: Add solid paragraph in new table cell (#2055)

* Content Model: Do color transform for entity when copy/paste (#2056)

* Content Model: Do color transform for entity when copy/paste

* Call normalizeContentModel

* Content Model: Paste plain text applies current format (#2057)

* Content Model: Paste plain text applies current format

* fix build

* Content Model: Fix a regression of shadow edit (#2058)

* Update versions

* Fix double import

* Double import 2

---------

Co-authored-by: Jiuqing Song <jisong@microsoft.com>
Co-authored-by: Bryan Valverde U <bvalverde@microsoft.com>
Co-authored-by: Júlia Roldi <juliaroldi@microsoft.com>
Co-authored-by: Julia Roldi <87443959+juliaroldi@users.noreply.github.com>
Co-authored-by: Andres-CT98 <107568016+Andres-CT98@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JiuqingSong JiuqingSong JiuqingSong approved these changes

@flyingbee2012 flyingbee2012 Awaiting requested review from flyingbee2012

@ianeli1 ianeli1 Awaiting requested review from ianeli1

@vinguyen12 vinguyen12 Awaiting requested review from vinguyen12

@juliaroldi juliaroldi Awaiting requested review from juliaroldi

@Andres-CT98 Andres-CT98 Awaiting requested review from Andres-CT98

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@BryanValverdeU @JiuqingSong