T/56: Block content in table cells. #97

jodator · 2018-08-01T09:50:52Z

Suggested merge commit message (convention)

Feature: Support block content inside table. Closes ckeditor/ckeditor5#3199.

BREAKING CHANGE: Removed table/commands/utils~getParentTable() method. Use table/commands/utils~findAncestor() instead.

Additional information

This PR requires also change in the engine: Fix: Selection on limit-block elements inside limit elements should b… ckeditor5-engine#1489 - blocks inside isLimit elements should be selectable.
Follow ups:

…verted child elements.

…k content is selected inside table cell.

…esent in the table.

# Conflicts: # tests/converters/downcast.js

# Conflicts: # src/tableediting.js

oskarwrobel · 2018-08-02T09:12:31Z

src/converters/table-post-fixer.js

-					writer.insertElement( 'tableCell', Position.createAt( table.getChild( rowIndex ), 'end' ) );
+					const tableCell = writer.createElement( 'tableCell' );
+					writer.insert( tableCell, Position.createAt( table.getChild( rowIndex ), 'end' ) );
+					writer.insertElement( 'paragraph', tableCell );


https://github.com/ckeditor/ckeditor5-table/pull/97/files#r206859198

oskarwrobel · 2018-08-02T09:27:09Z

src/converters/tablecell-post-fixer.js

+			// Check all nodes inside table cell on insert/remove operations (also other blocks).
+			const tableCell = entry.position && entry.position.parent;
+
+			if ( tableCell && tableCell.is( 'tableCell' ) ) {


tableCell.is( 'tableCell' ) 😉

I could name it possibleTableCell ;P

Or parent. But tableCell is fine too.

oskarwrobel · 2018-08-02T09:29:18Z

src/converters/upcasttable.js

+
+				const tableCell = conversionApi.writer.createElement( 'tableCell' );
+				conversionApi.writer.insert( tableCell, ModelPosition.createAt( row, 'end' ) );
+				conversionApi.writer.insertElement( 'paragraph', ModelPosition.createAt( tableCell, 'end' ) );


https://github.com/ckeditor/ckeditor5-table/pull/97/files#r206859198

oskarwrobel · 2018-08-02T09:57:31Z

src/converters/upcasttable.js

+
+			// Continue after inserted element.
+			data.modelCursor = data.modelRange.end;
+		}, { priority: 'normal' } );


Normal is a default value, so I would not define it. It's a detail but it's a few bytes of code more.

oskarwrobel · 2018-08-02T10:04:03Z

The code looks good to me. I found a few places where you can reduce the number of created operations (it should matter for insertRow and insertColumn commands where many operations are created at once).

jodator · 2018-08-03T08:31:13Z

Align Left does not work in the table cell. Align feature assumes that default alignment is left and but for table call is center.

Yeah - I've already pinged oleq about this.

…ommands/utils~getParentElement() to findAncestor().

Mgsy · 2018-08-06T08:01:51Z

Steps to reproduce

Go to the table block content sample.
Put the caret in the last cell of the first column.
Press Tab to select the image.
Press Delete.

Current result

The editor crashes.

Notes

The editor crashes after deleting every type of a block element. It won't crash if you'll use cut instead of delete.

Error

domconverter.js:280 Uncaught TypeError: Cannot read property 'parent' of null
    at DomConverter.viewRangeToDom (domconverter.js:280)
    at View.scrollToTheSelection (view.js:269)
    at Document.listenTo (delete.js:42)
    at Document.fire (emittermixin.js:196)
    at Document.DeleteObserver.document.on (deleteobserver.js:54)
    at Document.fire (emittermixin.js:196)
    at KeyObserver.fire (domeventobserver.js:96)
    at KeyObserver.onDomEvent (keyobserver.js:28)
    at ProxyEmitter.listenTo (domeventobserver.js:79)
    at ProxyEmitter.fire (emittermixin.js:196)

Browser: All browsers

Mgsy · 2018-08-06T08:16:50Z

Steps to reproduce

Go to the table block content sample.
Select whole block element in some cell.
Press Ctrl + X.

Current result

The table toolbar has disappeared.

Browser: Edge, Chrome, Safari

In Firefox, the toolbar disappears for a second and shows up again.

Mgsy · 2018-08-06T08:34:38Z

Steps to reproduce

Go to the table block content sample.
Put the caret in the last cell of the first column.
Press Tab to select the image.
Copy it.
Paste it to same cell with a content.
Put the selection outside of this cell.
Navigate to this cell with Tab.

Current result

Whole table's content is selected.

Browser: All browsers

Mgsy · 2018-08-06T10:35:58Z

Steps to reproduce

Go to the table block content sample.
Put the caret in the last cell of the first column.
Press Tab to select the image.
Repeat the previous step.

Current result

The selection moves to the beginning of the editor.

(edit) Reported here: https://github.com/ckeditor/ckeditor5-widget/issues/50)

jodator · 2018-08-07T14:54:19Z

After some testing and talks - I'd remove Image support in tables for this MVP as widget-ception causes some problems. Added a follow-up: #99.

jodator · 2018-08-08T14:38:41Z

Fixed:

The editor crashes after deleting every type of a block element. It won't crash if you'll use cut instead of delete.

In Firefox, the toolbar disappears for a second and shows up again.

jodator · 2018-08-08T14:41:19Z

@oskarwrobel & @Mgsy I've updated the PR with your comments i mind :)

jodator added 30 commits July 24, 2018 16:32

Fix: Downcast converter for table attributes should work with not con…

cac6afa

…verted child elements.

Test: Add proper tests for table schema in table editing tests.

8818f4d

Changed: Disallow table in table.

bdf8f45

Added: Wrap table cell in paragraph on enter.

bc5da5c

Added: Tab/Shift+Tab should properly move selection when part of bloc…

978b2f6

…k content is selected inside table cell.

Fix: InsertColumnCommand should work with block content.

8a8a0f2

Fix: InsertRowCommand should work with block content.

f68f902

Fix: InsertRowCommand should work with block content (#value only).

86350e7

Changed: Downcast conversion will work with a block content always pr…

4a6c8d6

…esent in the table.

Changed: Properly upcast table cell contents.

5320cd4

Added: A view post-fixer for spans in editing view.

50249ff

Test: Update table cells post fixer tests.

8479dac

Merge branch 'master' into t/56

63ad3a6

# Conflicts: # tests/converters/downcast.js

Added: Table post fixer should react on paragraph attribute change.

228d503

Changed: Extract tablecell-post-fixer.

fa55ebe

Tests: Add basic tests for the tablecell-post-fixer.

5b293ae

Fix: Wrong import path fo the paragraph plugin.

71388ef

Tests: Table tests for widgets should stub env.isEdge variable.

9e72b2a

Tests: Revert previous manual table test.

5275505

Tests: Create manual tests for block content inside table cell.

4edb2f8

Tests: Update manual tests description.

6e7ccb5

Tests: Dissalow table in table test for upcast converter.

5f0c4c2

Tests: Unify schema and conversion across tests.

5695b45

Tests: Add table integration tests.

9a428e9

Tests: Add tests for converting single paragraph with attributes.

10cf333

Other: Update dev-dependencies.

a450bd3

Tests: Fix package links in tests.

137dc0c

Tests: Update code coverage for tablecell post fixer.

aef2cf4

Merge branch 'master' into t/56

73d8c76

# Conflicts: # src/tableediting.js

Tests: Fix table post fixer tests for changes in tests utils.

3542841

oskarwrobel reviewed Aug 2, 2018

View reviewed changes

Remove default priority from converters.

2fea49b

jodator added 3 commits August 3, 2018 10:48

Unify creation of empty table cells with block content.

d9cbfe1

Changed: Removed commands/utils~getParentTable() method and renamed c…

d9f2790

…ommands/utils~getParentElement() to findAncestor().

Rename tableCell to parent as tableCell might be confusing.

9b2fe29

jodator added 5 commits August 7, 2018 16:56

Fix view selection on renamed elements by the table cell post-fixer.

d304c56

Disallow image in table.

3f1b811

Add more tests for table integration.

dd878ca

Fix package.json missing dev dependencies.

4c2d0b5

Refactor table cell post fixer.

c55942b

jodator and others added 4 commits September 3, 2018 12:43

Merge branch 'master' into t/56

3397df6

Remove image from block content manual test.

82845d9

Revert table cell contents alignment to default one (left).

bd5b81d

Merge branch 'master' into t/56

22be0ec

oskarwrobel merged commit cdf718e into master Sep 6, 2018

oskarwrobel deleted the t/56 branch September 6, 2018 11:49

This was referenced Oct 9, 2019

Cannot click-select image in table ckeditor/ckeditor5#3219

Closed

[Chrome] Fake selection over selected image inside table prevents tab key handling in table ckeditor/ckeditor5#4593

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

T/56: Block content in table cells. #97

T/56: Block content in table cells. #97

jodator commented Aug 1, 2018 •

edited by pomek

Loading

oskarwrobel Aug 2, 2018

oskarwrobel Aug 2, 2018

jodator Aug 2, 2018

oskarwrobel Aug 2, 2018

oskarwrobel Aug 2, 2018

oskarwrobel Aug 2, 2018

oskarwrobel commented Aug 2, 2018

jodator commented Aug 3, 2018

Mgsy commented Aug 6, 2018 •

edited

Loading

Mgsy commented Aug 6, 2018

Mgsy commented Aug 6, 2018

Mgsy commented Aug 6, 2018 •

edited by jodator

Loading

jodator commented Aug 7, 2018

jodator commented Aug 8, 2018 •

edited

Loading

jodator commented Aug 8, 2018

T/56: Block content in table cells. #97

T/56: Block content in table cells. #97

Conversation

jodator commented Aug 1, 2018 • edited by pomek Loading

Suggested merge commit message (convention)

Additional information

oskarwrobel Aug 2, 2018

Choose a reason for hiding this comment

oskarwrobel Aug 2, 2018

Choose a reason for hiding this comment

jodator Aug 2, 2018

Choose a reason for hiding this comment

oskarwrobel Aug 2, 2018

Choose a reason for hiding this comment

oskarwrobel Aug 2, 2018

Choose a reason for hiding this comment

oskarwrobel Aug 2, 2018

Choose a reason for hiding this comment

oskarwrobel commented Aug 2, 2018

jodator commented Aug 3, 2018

Mgsy commented Aug 6, 2018 • edited Loading

Steps to reproduce

Current result

Notes

Error

Mgsy commented Aug 6, 2018

Steps to reproduce

Current result

Mgsy commented Aug 6, 2018

Steps to reproduce

Current result

Mgsy commented Aug 6, 2018 • edited by jodator Loading

Steps to reproduce

Current result

jodator commented Aug 7, 2018

jodator commented Aug 8, 2018 • edited Loading

jodator commented Aug 8, 2018

jodator commented Aug 1, 2018 •

edited by pomek

Loading

Mgsy commented Aug 6, 2018 •

edited

Loading

Mgsy commented Aug 6, 2018 •

edited by jodator

Loading

jodator commented Aug 8, 2018 •

edited

Loading