ckeditor / ckeditor5

Powerful rich text editor framework with a modular architecture, modern integrations, and features like collaborative editing.
https://ckeditor.com/ckeditor-5
Other
9.64k stars 3.71k forks source link

Support for Tables with Multi-Level Headers / Allow <tbody> <th> in any column #15022

Closed jakegibs617 closed 7 months ago

jakegibs617 commented 1 year ago

Description of the new feature

As seen in https://www.w3.org/WAI/tutorials/tables/multi-level/ Screenshot 2023-09-18 at 2 24 24 PM Screenshot 2023-09-18 at 2 29 01 PM

What is the expected behavior of the proposed feature? There are lots of examples where complex multi-level headers are needed. And we cannot do this currently with CKEditor5. We need the ability to assign table header to individual cells without changing other cells. Below is a draft of what the unit test for this could look like:

/**
 * @license Copyright (c) 2003-2023, CKSource Holding sp. z o.o. All rights reserved.
 * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
 */

/* global document */

import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
import View from '@ckeditor/ckeditor5-ui/src/view';
import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import global from '@ckeditor/ckeditor5-utils/src/dom/global';
import Table from '../src/table';
import TableToolbar from '../src/tabletoolbar';
import { expect } from 'chai';

describe.only('TableUI', () => {
    let newEditor, editorElement;

    beforeEach(() => {
        editorElement = global.document.createElement('div');
        global.document.body.appendChild(editorElement);

        return ClassicTestEditor.create(editorElement, {
            plugins: [Table, TableToolbar, Paragraph],
        }).then((editor) => {
            newEditor = editor;
            const button = new View();

            button.element = global.document.createElement('div');

            newEditor.editing.view.isFocused = true;
            newEditor.editing.view.getDomRoot().focus();
        });
    });

    afterEach(() => {
        editorElement.remove();
        return newEditor.destroy();
    });

    describe.only('Table headers with colspans', () => {
        it('should output correct table structure with headers and colspans', () => {
            // Step 1: Insert the table into the model
            setModelData(
                newEditor.model,
                '<table headingRows="2">' + // Two header rows
                    '<tableRow>' +
                    '<tableCell><paragraph></paragraph></tableCell>' +
                    '<tableHeaderCell><paragraph>studio</paragraph></tableHeaderCell>' +
                    '<tableHeaderCell><paragraph>apt</paragraph></tableHeaderCell>' +
                    '<tableHeaderCell><paragraph>condo</paragraph></tableHeaderCell>' +
                    '</tableRow>' +
                    '<tableRow>' +
                    '<tableHeaderCell colspan="4"><paragraph>NYC</paragraph></tableHeaderCell>' +
                    '</tableRow>' +
                    '<tableRow>' +
                    '<tableHeaderCell><paragraph>1 Bed</paragraph></tableHeaderCell>' +
                    '<tableCell><paragraph>11</paragraph></tableCell>' +
                    '<tableCell><paragraph>20</paragraph></tableCell>' +
                    '<tableCell><paragraph>25</paragraph></tableCell>' +
                    '</tableRow>' +
                    '<tableRow>' +
                    '<tableHeaderCell><paragraph>2 Bed</paragraph></tableHeaderCell>' +
                    '<tableCell><paragraph>-</paragraph></tableCell>' +
                    '<tableCell><paragraph>43</paragraph></tableCell>' +
                    '<tableCell><paragraph>52</paragraph></tableCell>' +
                    '</tableRow>' +
                    '<tableRow>' +
                    '<tableHeaderCell colspan="4"><paragraph>Boston</paragraph></tableHeaderCell>' +
                    '</tableRow>' +
                    '<tableRow>' +
                    '<tableHeaderCell><paragraph>1 Bed</paragraph></tableHeaderCell>' +
                    '<tableCell><paragraph>13</paragraph></tableCell>' +
                    '<tableCell><paragraph>21</paragraph></tableCell>' +
                    '<tableCell><paragraph>22</paragraph></tableCell>' +
                    '</tableRow>' +
                    '</table>'
            );
            const editorData = newEditor.getData();

            expect(editorData).to.equal(
                '<figure class="table"><table><tbody><tr><td>&nbsp;</td><th>studio</th><th>apt</th><th>condo</th></tr><tr><th colspan="4">NYC</th></tr><tr><th>1 Bed</th><td>11</td><td>20</td><td>25</td></tr><tr><th>2 Bed</th><td>-</td><td>43</td><td>52</td></tr><tr><th colspan="4">Boston</th></tr><tr><th>1 Bed</th><td>13</td><td>21</td><td>22</td></tr></tbody></table></figure>'
            );
        });
    });
});

Currenlty this is failing:

FAILED TESTS:
  TableUI
    Table headers with colspans
      ✖ should output correct table structure with headers and colspans
        Chrome 117.0.0.0 (Mac OS 10.15.7)
      Element 'tableHeaderCell' was not allowed in given position.
      Read more: https://ckeditor.com/docs/ckeditor5/latest/support/error-codes.html#error-Element 'tableHeaderCell' was not allowed in given position.
      Error: Element 'tableHeaderCell' was not allowed in given position.
          at UpcastDispatcher.<anonymous> (packages/ckeditor5-engine/src/dev-utils/model.ts:476:10 <- entry-point.1730408780.js:29262:13)
          at UpcastDispatcher.fire (packages/ckeditor5-utils/src/emittermixin.ts:241:31 <- entry-point.1730408780.js:104235:35)
          at UpcastDispatcher._convertItem (packages/ckeditor5-engine/src/conversion/upcastdispatcher.ts:264:9 <- entry-point.1730408780.js:27347:12)
          at UpcastDispatcher._convertChildren (packages/ckeditor5-engine/src/conversion/upcastdispatcher.ts:314:24 <- entry-point.1730408780.js:27377:27)
          at Object.convertChildren (packages/ckeditor5-engine/src/conversion/upcastdispatcher.ts:174:64 <- entry-point.1730408780.js:27296:65)
          at UpcastDispatcher.<anonymous> (packages/ckeditor5-engine/src/dev-utils/model.ts:488:17 <- entry-point.1730408780.js:29268:19)
          at UpcastDispatcher.fire (packages/ckeditor5-utils/src/emittermixin.ts:241:31 <- entry-point.1730408780.js:104235:35)
          at UpcastDispatcher._convertItem (packages/ckeditor5-engine/src/conversion/upcastdispatcher.ts:264:9 <- entry-point.1730408780.js:27347:12)
          at UpcastDispatcher._convertChildren (packages/ckeditor5-engine/src/conversion/upcastdispatcher.ts:314:24 <- entry-point.1730408780.js:27377:27)
          at Object.convertChildren (packages/ckeditor5-engine/src/conversion/upcastdispatcher.ts:174:64 <- entry-point.1730408780.js:27296:65)

Error: Karma finished with "1" code.

If you'd like to see this feature implemented, add a 👍 reaction to this post.

Witoso commented 1 year ago

Thanks for the detailed request!

wimleers commented 1 year ago

Indeed, thanks!

Previously discussed in Drupal Slack: https://drupal.slack.com/archives/C01GWN3QYJD/p1695044466385619

Witoso commented 1 year ago

Information from https://github.com/ckeditor/ckeditor5/issues/15171

Summary

When I paste in CKE5 Source mode the following table, and switch out of source mode and then back into source mode, the <th> is converted to a <td>.

<table>
    <tbody>
        <tr>
            <td><a href="#">edit</a></td>
            <th scope="row">I AM THE IMPORTANT CELL</th>
            <td>things</td>
            <td>more things</td>
        </tr>
    </tbody>
</table>

Context

It appears that currently a <th> must START the rows, and you cannot have a <td> followed by a <th>.

However, this is NOT a requirement of the HTML5 spec. In fact, the example in the specification uses a <th> after a <td>.

  <tr> <td> <th scope=rowgroup> Cats <td> <td>

📝 Detailed reproduction steps

This bug can be confirmed without the Source mode button by using the Table widget balloon toolbar operations in the Classic Editor example.

  1. Open the Classic Editor and clear out all contents.
  2. Use the toolbar to insert a table with 4 rows and 2 columns.
  3. Enter data in all cells.
  4. Select the second column, then select the "Column" icon from the Table widget balloon toolbar. Then enable the Header column toggle.

✔️ Expected result

Only the selected 2nd column becomes a header column (visually indicated by bold text and a light gray cell background color).

❌ Actual result

Both the 1st and 2nd column become header columns.

Video recording

https://github.com/ckeditor/ckeditor5/assets/243532/2ca120e6-4ea6-4c46-b1ee-e44360f4de27

Witoso commented 11 months ago

Rel: #14911

Witoso commented 8 months ago

Actually, let's merge to #14911.