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.54k stars 3.7k forks source link

Some classes are too generic, non-descriptive, and incompatible with certain CSS frameworks #15765

Open justin-oh opened 9 months ago

justin-oh commented 9 months ago

Outlining the Issue

I'm proposing the class attributes for various elements created by the editor be changed. Specifically the <figure> elements, but there could be others I'm not familiar with.

There are at least 3 different <figure> elements that I'm aware of:

  1. <figure class="image"></figure>
  2. <figure class="media"></figure>
  3. <figure class="table"></figure>

Below I will explain specifically why these classes should be improved.

Incompatibility

The markup <figure class="table"></figure> is incompatible with Bootstrap. Bootstrap expects the .table class to be applied to something that behaves like a table, not something that contains a table. This and the other .media and .image classes could be incompatible with other CSS frameworks.

Too Generic

Classes like .table, .media, and .image are way too generic to be used in 3rd-party code that does nothing with said classes other than apply them to an element. There is a reason why all the classes for laying out the CKEditor itself are prefixed with ck: to avoid naming collisions.

Non-descriptive

As I alluded to earlier, the class .table is expected to be on something that behaves like a table. Same with .image and .media. If you want to describe something that contains a table, you might use a class like .figure-with-table. or .figure-table as those better describe the element.

Proposed Solution

A solution for the <figure> elements would be to simply add figure- to the existing classes, and possibly add an additional .figure class. So <figure class="table"> becomes <figure class="figure-table"> or <figure class="figure figure-table">.

This same concept can be applied to other elements whose classes fit the above criteria.

Witoso commented 9 months ago

Thanks for the report! For the background, this is the research we were following: https://ckeditor.github.io/editor-recommendations/features/table.html

Would you mind providing further details?

is incompatible with Bootstrap

You mean the Bootstrap's styles bleeding into the editor when writing or on the already published content?

incompatible with other CSS frameworks.

Are you aware of any specific ones?

justin-oh commented 9 months ago

This is the Bootstrap documentation for tables: https://getbootstrap.com/docs/5.3/content/tables/. Bootstrap expects the following markup:

<table class="table">
</table>

If a developer was loading CKEditor with the CDN, they would get this markup:

<figure class="table">
  <table>
  </table>
</figure>

Of course there are ways for the developer to get the .table class onto the <table> element, but that's extra work which may cause the developer to want to use a different WYSIWYG. You could see how that could cause issues by styles being applied to the .table class on both the <figure> and <table> elements.

I'm not aware of any other frameworks at the moment, but it should be clear how generic classes like .table, .image, .media, etc. could conflict with other frameworks or even existing styles on a site.

For what it's worth, the CKEditor recommendations is what suggests the <figure> element needs a class. I don't see anything in the HTML5 standard that says the <figure> element should have a class.

justin-oh commented 8 months ago

A simple, backwards compatible solution is to offer the developer an easy way to provide their own classes:

const editor = await ClassicEditor.create( element , {
  classNames: {
    figureImage: 'figure-image', // defaults to 'image'
    figureMedia: 'figure-media', // defaults to 'media'
    figureTable: 'figure-table', // defaults to 'table'
  }
});

The above option structure might be very different in practice, but you should get the idea.

Witoso commented 8 months ago

For what it's worth, the CKEditor recommendations is what suggests the <figure> element needs a class. I don't see anything in the HTML5 standard that says the <figure> element should have a class.

True, those recommendations are HTML standards, and our thoughts after of building editors for 20y. I discussed this topic internally, one argument that also appeared, was producing clean HTML without connection to any editor-specific names.

This would be a big breaking change, as those class names also serve a purpose in loading the data to our model layer. I will keep this issue open to gather more feedback, but there will be no immediate action on our side.