-
Notifications
You must be signed in to change notification settings - Fork 113
Multiline tables with block node content #7523
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.
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
Conversation
b6526cb to
c619227
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
c619227 to
b18ab8b
Compare
ac21086 to
8e40758
Compare
|
@mejo- Did you figure out what prevents nesting a table in a table right now? |
Not exactly, but I guess it's rather a broken |
|
Will this fix as well, rich previews added with the smart picker ? |
Just wanted to say that I would love this being broken 🫠 |
bba037a to
48ccc4c
Compare
12cb94a to
30e25f0
Compare
| if (!tablePaste) { | ||
| return slice | ||
| } | ||
|
|
||
| if ( | ||
| slice.content.childCount === 1 | ||
| && slice.content.firstChild?.type.name === 'table' | ||
| ) { | ||
| const tableChild = slice.content.firstChild.firstChild | ||
| if ( | ||
| (tableChild.childCount === 1 | ||
| && tableChild.type.name === 'tableRow') | ||
| || tableChild.type.name === 'tableHeadRow' | ||
| ) { | ||
| const rowChild = tableChild.firstChild | ||
| if ( | ||
| (rowChild.childCount === 1 | ||
| && rowChild.type.name === 'tableCell') | ||
| || rowChild.type.name === 'tableHeader' | ||
| ) { | ||
| return new Slice(rowChild.content, 0, 0) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| console.warn('Nested tables are not supported') | ||
| alert( | ||
| t( | ||
| 'text', | ||
| 'A table was pasted into a table. Nested tables are not supported.', | ||
| ), | ||
| ) | ||
|
|
||
| const newSlice = new Slice(Fragment.empty, 0, 0) | ||
| return newSlice |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need to rule out multi cell pastes?
Would that effectively lead to nested tables or just paste the cells at the current cursor position?
The latter seems like it might be useful and if it's already the current behavior would be nice to keep it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I gave it a try by selecting two cells and pasting them and it resulted in a full new table being added to the cell, i.e. a nested table. If that's what you mean (also what we talked about last week), then I fear that's not possible out of the box and not working already now.
max-nextcloud
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good code wise. Have not tested it yet though.
Allow paragraphs, lists, code blocks and images in table cells for now. Signed-off-by: Jonas <[email protected]>
Signed-off-by: Jonas <[email protected]>
Signed-off-by: Jonas <[email protected]>
Table cells with lists or code blocks are hardcoded to left alignment though, as everything else would break markdown parsing later. Signed-off-by: Jonas <[email protected]>
Signed-off-by: Jonas <[email protected]>
Signed-off-by: Jonas <[email protected]>
Dashes in nodes names are not nice and this PR changes the editor schema anyway. Signed-off-by: Jonas <[email protected]>
Unnecessary and breaks serializing e.g. nested lists. Signed-off-by: Jonas <[email protected]>
Signed-off-by: Jonas <[email protected]>
Also add tests for tables with complex nested structures in their cells. Signed-off-by: Jonas <[email protected]>
With multiline table cells, we don't want to jump into next table row any longer when pressing <Return> two times in a row. Signed-off-by: Jonas <[email protected]>
Signed-off-by: Jonas <[email protected]>
Single line block node cell content is not valid markdown. Signed-off-by: Jonas <[email protected]>
The editor schema changed as table cells now support block node content. Signed-off-by: Jonas <[email protected]>
Check pasted content for possibly nested table when pasting to a table cell. Show an alert if nested table is detected. Add some extra logic to detect if just one table cell got selected and copied, which results in a table with one row and cell in the clipboard. Unwrap the cell content in this case and paste it. Add `filterTransaction` to prevent applying a transaction that would result in nested tables. This is meant as an extra low-level protection against nested tables. Signed-off-by: Jonas <[email protected]>
We don't support headerless tables in the editor schema anyway, so let's not add partial support for them that results in markdown being altered. Signed-off-by: Jonas <[email protected]>
Signed-off-by: Jonas <[email protected]>
Signed-off-by: Jonas <[email protected]>
30e25f0 to
3258953
Compare
Signed-off-by: Jonas <[email protected]>
3258953 to
b625cbb
Compare
📝 Summary
🖼️ Screenshots
🚧 TODO
block+as content for tableCell. For some reason, allowing blockquotes results in all cell rows being serialized to markdown as blockquote. I didn't find out yet why this is the case (fixed magically, probably something upstream in Tiptap)2x return used to create/jump to next row. This is currently broken. Do we want it back?(we don't)Optionally remove our custom newline special-handling with(let's keep it to not break backwards-compatibility when parsing existing tables)<br>in table cellsfilterTransaction(as suggested here)🏁 Checklist
npm run lint/npm run stylelint/composer run cs:check)