CKEditor 5 Inline: remove margins added to :first-child and :last-child by CKEditor stylesheets

[CaelanStewart]

I'm using CKEditor 5 Framework - more specifically, the InlineEditor.

However, CKEditor's stylesheets also include the following:

.ck.ck-editor__editable_inline > *:first-child {
    margin-top: var(--ck-spacing-large);
}

.ck.ck-editor__editable_inline > *:last-child {
    margin-bottom: var(--ck-spacing-large);
}

As you might guess, this interferes with the stylesheets applied by my app code.

I am unable to find an option to disable this in CKEditor. I'm also in a position where I cannot expect the specificity of the stylesheets for the elements InlineEditor is instantiated on to be more than that of CKEditor's stylesheets.

I've also tried setting CKEditor's CSS variables to values that are invalid for use with margin properties. That doesn't work - it just seems to default to zero in such cases.

I don't want to have to ignore CKEditor's stylesheets in the build system and then make a copy of them myself just to remove those statements. Nor do I want to remove the classes added by CKEditor after initialisation. That feels like a nasty hack.

If there's no built-in way of doing this, I'd like to propose an option or some easy mechanism to disable it.

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

[Mgsy]

@oleq, can you take a look at it?

[oleq]

These are because of https://github.com/ckeditor/ckeditor5/issues/3385 and https://github.com/ckeditor/ckeditor5/issues/847.

They are sane defaults that (along with the default padding) work for 99% of integrations. This way people who run the editor for the first time don't need to worry about writing CSS because these styles provide a decent look and user experience.

I am unable to find an option to disable this in CKEditor. I'm also in a position where I cannot expect the specificity of the stylesheets for the elements InlineEditor is instantiated on to be more than that of CKEditor's stylesheets.

I'm not sure what you mean by this. All you need to do is to add your own styles that negate/disable the defaults. For instance:

.my-app .ck.ck-editor__editable_inline > *:first-child {
    margin-top: 0;
}

.my-app .ck.ck-editor__editable_inline > *:last-child {
    margin-bottom: 0;
}

[CaelanStewart]

@oleq - I concede that they are sane defaults. (Edit: thinking it through, I'm not even sure that is the case. The whole point of an inline editor is for it adhere to already defined stylesheets of wherever it is instantiated. Why would margins not already be defined? Especially if one uses a CSS framework such as Bootstrap - not using one would be not be considered 'sane')

However, what I need is a mode which assumes that all paddings/margins have been added already.

Let me explain my use case.

I'm working on a document editing platform. It has a whole bunch of templates. Each template has its own Vue components and associated stylesheets. We used to require all editable fields to be edited via a sidebar, with actual inputs that map to specific outputs in the templates.

We're now implementing in-page editing, and trying out CK Editor 5 Framework, as it seems most suited to our needs due to its integration with build systems, and ES6 based codebase, and its framework disposition.

Because the specificity of the selectors in the specified snippet is moreso than that of the selectors in our template stylesheets, it would require us to retrospectively amend stylesheets for all previous templates. This is not feasible.

We need CKEditor to adhere to as strict a non-interference policy as is practical. We've had to make a few amendments to previous templates, but so far those have been trivial due to the abstraction of field output in our templates.

We also validate output, checking that certain content areas do not exceed their defined bounds.

I hope you can see that, in such a case as this, that your stylesheets overriding the margins of our existing template stylesheets, breaking certain forms of output validation for default content in some cases, due to increased margin sizes increasing the total size of the content, is problematic. Especially where each template design is unique, with unique typography, unique proportions etc.

The simplest solution is to prevent CKEditor from adding its own margins in such a case where there are already margins defined for selectors with lesser specificity.

I can imagine this design decision also affecting other similar use cases.

[CaelanStewart]

@oleq - any word? I've explained above the necessity of a feature like this for what could be a fairly broad range of inline integrations.

I can hack this, but you re-add the class that I need to remove on focus, so I need to keep removing the class, so I don't need to ignore CKEditor's built-in stylesheets and include my own edited version with those statements removed.

For the use case I describe, along with any other similar use case - think somebody building a page builder for a CMS - then all existing themes/stylesheets have their paddings/margins overridden because the specificity of the selector which adds the margin is higher, and therefore when editing inline, it will not be exactly WYSIWYG, in other words the inline aesthetic will not map 1:1 with the eventual output to the end-user when no inline editor instance is instantiated.

For these purposes, CKEditor is a pain because you now have to hack it so that the stylesheets are not applied. But, CKEditor, from the options available, seems to be the best editor, despite this one relatively-minor design decision.

I would even be willing to look into a writing a PR.

Just an option to that is false by default called something along the lines of nonInterferenceMode which would then apply absolutely no stylesheets to the inline editor element, or its children.

[Lusito]

I have the same issue.. trying the inline editor and it overrides the styles I have. The whole point of an inline editor is to show you how it finally looks. These styles (margin and padding) break how it looks.

Overriding these styles is no option, as I don't know what these styles might have been originally (theme can vary).

The only way I currently see to make this work is to change the content of the style tag after it has been inserted and replace the bad parts. That's not a very stable solution.

Edit: Wow, it even changes colors, font sizes and borders.

[CaelanStewart]

@Lusito, for our case, it was easier to remove the ck class from the inline editor instance element. It will be added again on focus/blur, however, you can hook onto focus change event and then remove the class again. I'm not sure if there are other state changes which trigger an update of the class list on the instance element, but you should be able to hook into any state change and remove the offending class again.

I haven't found any issues rooted in its removal thus far. No styling at all is applied except to CKEditor's toolbar, which is unaffected by the class removal from the editor element, and it is located in a different position in the DOM.

This way I don't have to ignore their stylesheets and roll my own redacted copy.

Plus, it is resistant to future updates. I won't have to keep my redacted copy of CKEditor's stylesheets in sync with package version changes.

If you create an abstraction layer between the InlineEditor and the depending code, and CKEditor add support for non-interference mode, you can then remove the class removal code, and simply set the corresponding option, without updating all consuming code.

[oleq]

As @Lusito pointed out, the problem is broader than first/last-child margins. It's about regaining full control over the editable styles.

We know that our default styles won't satisfy all integrations and some (more uncommon) will have to do something extra to get things done. But above all, our editor must work out-of-the-box for most of them and these paddings/margins are just for that, regardless of the UI framework in use (or no framework at all).

We cannot get rid of these styles now because that would break thousands of integrations that depend on them. And honestly, we don't even want to do that because the vast majority of our users and customers are actually OK with them (this is the first issue since those styles were implemented 2 years ago).

If you want to regain control over the styles of the editable, you should get rid of CSS classes that bring these styles. Getting rid of .ck-editor__editable_inline will remove padding and :first/last-child styles (and everything brought by editorui.css).

InlineEditor
        .create( ..., {
            // ...
        } )
        .then( editor => {
            editor.editing.view.change( writer => {
                const viewEditableRoot = editor.editing.view.document.getRoot();

                writer.removeClass( 'ck-editor__editable_inline', viewEditableRoot );
            } );
        } )
        .catch( err => {
            console.error( err.stack );
        } );

If you don't want our default content styles, get rid of .ck-content in the same way.

If you want some subset of content styles, copy them from the official guide and paste them under a different class that you can add via writer.addClass( 'my-content-class', viewEditableRoot );

The only task on our side, in my opinion, comes down to moving some styles like padding and :first/last-child from under .ck-editor__editable_inline to .ck-content. This way removing just a single class will give developers back full control over the editable.

I hope this will help you out.

[CaelanStewart]

@oleq - "the problem is broader than first/last-child margins". I had gathered that from my integration experience.

I found that removing the ck class was preferable, so it doesn't even add a border/focus state. I'm handling that myself. Not encountered any negative side-effects of that decision yet.

However, as I suggested, to which you have not replied, why not just add an option 'nonInterferenceMode', which is false by default, and change the selectors in your stylesheets accordingly, such as .ck-editor__editable_inline.ck-default-style ...?

Seems like quite a simple change, and not unreasonable to open up integration to what is potentially a fairly broad range of applications.

Adding and removing classes at points feels like a really nasty hack.

[CaelanStewart]

@oleq - also, it would be interesting to find out how many people have encountered this issue, but for them they had control over all of their stylesheets, and just increased the specificity of their selectors to exceed that of CKEditor's stylesheets? Or, God forbid, just add !important!

Could you really call that "happy with the feature", or "no problems", if they've just quickly plastered over it?

How many big time CMS software vendors have adopted CKEditor 5? Usually not as soon as it is released. Good official and community support tends to be a requirement in any such setting, and therefore it is less like that any big time software vendors/open source projects would adopt it without it being in the wild for a least some decent length of time to garner up enough issues and general support (e.g. StackOverflow Answers, and GitHub issues).

Perhaps CKEditor 5 Framework hasn't been around long enough to garner enough users for the full extent of these issues is discovered?

What if I were representing WordPress, and they wanted to use CKEditor 5 Framework for a page builder interface? They would have the same problem, and would likely end up posting an issue. Well, mind you, some of the hacky nature of WordPress that I've seen, I'm not so certain. Regardless, the point I make is clear.

[oleq]

However, as I suggested, to which you have not replied, why not just add an option 'nonInterferenceMode', which is false by default, and change the selectors in your stylesheets accordingly, such as .ck-editor__editable_inline.ck-default-style ...?

We don't write styles in JS, neither we build styles in JS and we don't have control over them in the runtime using JS. They are static CSS files, parsed by PostCSS when building (webpacking) and loaded using style-loader. I see no option to do what you suggested at this moment and implementing anything like this would mean a major refactoring of our architecture.

Could you really call that "happy with the feature", or "no problems", if they've just quickly plastered over it?

As you correctly pointed out, we don't know that. And there's no way to learn that other than by reading the feedback developers provide. I'm sorry but we cannot change key pieces of a framework blindly hoping the changes will outweigh possible disasters. We need significant data to justify our actions.

What if I were representing WordPress, and they wanted to use CKEditor 5 Framework for a page builder interface?

They wouldn't do that in public GitHub comments, that's for sure 😉

---

Anyway, I'm closing this issue and creating a follow-up about styles refactoring. It will help developers get rid of default editable styles in a single Writer call.

[CaelanStewart]

@oleq - obviously you don't write your stylesheets in JS - that's not what I'm saying.

However, what you CAN do is add an option into CKEditor, so that when the option is true, an extra class is added to the editor instance element, and then you can edit the selector in your stylesheets to require that class be present on the editor instance element in order for the margins/fonts etc. to be applied.

Does that make sense?

[oleq]

@CaelanStewart If I understand you correctly, you want editable styles we're talking about to be opt-in, right? If so, then I'm afraid we cannot do that for the reasons I mentioned before: the editor must be ready to use out–of–the–box for the majority of integrations. It is one of our top priorities.

At the same time, it is possible easily to opt–out from these styles and we can make it even simpler.

[CaelanStewart]

@oleq - no I'm saying there needs to be an option to to OPT OUT, which is FALSE by default, so as to not break compatibility.

For clarity, I agree completely that you can't change the default settings! I'm with you on that.

I apologise if I'm not explaining myself properly here - I'm trying my best to be clear and explicit!

Just an option I can pass to the InlineEditor instance which adds a class (or doesn't add - whatever you decide is the best way to implement it) to the instance element, which is then referenced (or not referenced) in the selector that adds in the offending styles.

This way we don't have to override the editor stylesheets, we don't need to modify our webpack setup to use a different CSS file rather than the one bundled with CKEditor, AND so we don't have to remove classes from the instance element after instantiation and on focus/blur.

[Lusito]

@oleq I tried your approach, but it only works partially.. a lot of styles still in tact. I have this script, which seems to cover all cases I can see out of the box. Not sure if there are other elements, which the default toolbar doesn't offer:

(typescript, i.e. remove the type definitions in js)

const EXCLUDED_CK_CLASSES = [
    ".ck-editor__editable_inline",
    ".ck-widget_selected",
    ".ck-editor__editable",
    ".ck-editor__nested-editable",
    ".ck-content"
];

function removeCkStyles(doc: Document) {
    const stylesheets = [...doc.styleSheets].filter((s) => !s.href);
    for (const stylesheet of stylesheets) {
        const sheet = stylesheet as CSSStyleSheet;
        for (let i = sheet.cssRules.length - 1; i >= 0; i -= 1) {
            const rule = sheet.cssRules[i] as CSSStyleRule;
            if (rule.selectorText && EXCLUDED_CK_CLASSES.some((clazz) => rule.selectorText.includes(clazz)))
                sheet.deleteRule(i);
        }
    }
}

Run this after(!) the ckeditor script tag has finished loading.

The script looks over all stylesheets in the supplied document, which does not have an href attribute (i.e. ignores external stylesheets) and then removes all rules, which contain a certain class in their selector. I imagine this is not precise and it might have some false positives, but it works better than your approach, even if I remove the same classes.

It does require you to re-add some styles, but it's a lot better than before.

Edit: It seems to break the vertical position calculation of the image balloon tip for some reason. This happens when this margin-top is no longer existing:

.ck.ck-editor__editable_inline>:first-child

[Reinmar]

However, what you CAN do is add an option into CKEditor, so that when the option is true, an extra class is added to the editor instance element, and then you can edit the selector in your stylesheets to require that class be present on the editor instance element in order for the margins/fonts etc. to be applied.

Does that make sense?

It does make sense. But it does not mean that it's the best option and that it's a reasonable option in our case.

Every single option that we'll ever add stay with us for years. CKEditor 3-4 was developed with an "let's add an option for that" attitude and it has over 200 of them and it's a mess to maintain. 

In addition to that, the option that you're proposing is just more code to add. That's another thing that we need to take care of – the code size.

What @oleq proposed is very similar to the option that you're proposing. But instead of adding more code, it uses CKE5 API to remove a class that's already in use and on which all the content styles are based (or will be once #6405 is resolved). This is perhaps less elegant as it requires more code on the integration side, but maintainability and elegance are relative measures and here the elegance is in the fact that no changes in the core are needed.

[CaelanStewart]

@Reinmar - that's fair enough, I'll concede to that reasoning.

@Lusito - that's going to be a tricky way to solve the problem effectively, and reliably. I suggest you scrap that method and just remove the classes from the container in the manner that @oleq has suggested, but also remove the ck class, as that adds borders and a few other things.

[Lusito]

Thanks @CaelanStewart, removing the ck class in addition seems to remove all things I can see at this point (even though it also removes a bit more styling, which will have to be re-added manually I guess).

There's still the issue of the misplaced balloon tip for the image. Any idea how to fix this? When selecting an image, an extra balloon tip appears "full size" vs "side image". This balloon appears normally below the image, but when all the classes are removed as you propose, this balloon appears behind the normal toolbar. Its top position calculation is off.

[Lusito]

Once we figured all of this out, do you think it would be possible to add the gathered information somewhere in your documentation?

[CaelanStewart]

@Lusito - Yeah I had to re-style the selection handle for the table widget after removing that class, but I was going to do that to bring it into the style of the rest of the app anyway.

Regarding the Image widget, I've not enabled that, and don't need to right now, so I have nothing to suggest there.

To be honest, the class removal trick, while simpler in one sense (just removing a class), potential complexity increases in another, in that we have to re-style certain components of widgets (like the table).

And if a future update comes along which changes the classes certain stylesheet selectors apply to, then the class-removal hack would still need to be updated, just as a script that mutates the stylesheets on the page would equally require amendments in such a case.

It feels like, overall, the class-removal trick would be a little easier to get working again after updates, and might be more resilient to future change around the code in the application.