ckeditor5 icon indicating copy to clipboard operation
ckeditor5 copied to clipboard

Set className for outermost Editor wrapper

Open tony opened this issue 4 years ago • 12 comments

📝 Provide a description of the improvement

When using a Super build of Balloon and Classic editor, we want to have custom CSS Classes to namespace the Styling.

Workaround causes flicker on launch and is fragile

In our case, using a workaround via callback/hooks in @ckeditor/react hook's:

is fragile/brittle since it relies on events to assure our theming prefix is always attached.

Balloon editor / toolbar: A wrapper div wouldn't work as its rendered outside the editor region

Classic editor:

  • The option to use an extraneous div / dom node isn't desirable, for the same reason Fragments exist, and the Editor UI already has an outermost <div.

Ideas

  • For builds, create a reference / convention for id and class of the outermost div, Note: This varies by the build:
    • Classic Editor: module:editor-classic/classiceditorui~ClassicEditorUI#element ClassicEditor#element
    • module:editor-balloon/ballooneditorui~BalloonEditorUI#element editor.ui.element
  • Additionally, portal'd or separate containers managed by CKEditor should also add these CSS class, so they're prefixed and can be themed by that namespace

📃 Other details

  • Browser: …
  • OS: …
  • CKEditor version: 27.0.0 and 27.1.0 (possibly before/beyond)
  • Installed CKEditor plugins: ckeditor classic, balloonblock

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

tony avatar Apr 21 '21 20:04 tony

There are 4 places where this class should be set:

  • on the outermost element of the classic editor UI
  • on the editing root 
    • mostly for inline, block, and document editors because their UI has no main container, 
    • does no harm to the classic editor although it sort of duplicates the class set on the outermost UI element
  • on the body collection container 
    • for all floating UI, e.g. balloons, toolbars, etc.
  • manually on the toolbar container of the document editor
    • this could be automated by setting the class on its toolbar element, not sure whether this is the right thing to do, though

A workaround (first 3 cases) could be a simple plugin:

ClassicEditor
 .create( document.querySelector( '#editor' ), {
  plugins: [ ..., EditorClassPlugin ], 
  // ...
  editorClass: 'my-custom-class'
 } )
 .then( editor => {
  window.editor = editor;
 } )
 .catch( err => {
  console.error( err.stack );
 } );

function EditorClassPlugin( editor ) {
 const className = editor.config.get( 'editorClass' );

 editor.ui.on( 'ready', () => {
  // For all balloons and popups to inherit from.
  editor.ui.view.body._bodyCollectionContainer.classList.add( className );

  // Note: Balloon editor doesn't have one.
  if ( editor.ui.view.element ) {
      editor.ui.view.element.classList.add( className );
  }
 } );

 // For the editing root. In the Classic editor, it slightly duplicates with the class set on 
 // editor.ui.view.element because editor.ui.view.element contains the editing root. In the Balloon editor, 
 // which does not have the UI container (view), this makes perfect sense, though.
 editor.editing.view.change( writer => {
  writer.addClass( className, editor.editing.view.document.getRoot() );
 } );
}

but I guess this should be handled internally by the editor especially that editor.ui.view.body._bodyCollectionContainer is private.

oleq avatar May 06 '21 12:05 oleq

@oleq Regarding this issue: what do you think should happen next since the first 3 aspects of this are solved by the Plugin posted above?

tony avatar May 06 '21 14:05 tony

As I wrote in the PR https://github.com/ckeditor/ckeditor5/pull/9540#issuecomment-836325661, let's keep this issue open and see how popular this is.

oleq avatar May 10 '21 07:05 oleq

Would love to see it in the core.

Our implementation involves possibility of having several editor on page for different blocks of content, and we want to be able to style the content differently in those.

(The plugin works great)

intoeetive avatar Aug 25 '22 08:08 intoeetive

Considering using something like this plugin to set z-indexes on the balloons (is that the right word?) for using Link plugin inside a MUI modal.

Works great, but I'd love to see a way to do this that doesn't rely on presumably private _bodyCollectionContainer variable.

ChristopherChudzicki avatar Aug 26 '22 19:08 ChristopherChudzicki

@oleq https://github.com/ckeditor/ckeditor5/issues/9539#issuecomment-833491911 plugin has an issue w/ v37.0.0

_bodyCollectionContainer is private

https://github.com/ckeditor/ckeditor5/blob/fba274b35d542764148c1996a74c91df01e4151b/packages/ckeditor5-ui/src/editorui/bodycollection.ts#L44

I can make a new issue if you wish

cc: @ChristopherChudzicki in re: the variable being private

tony avatar Apr 05 '23 14:04 tony

@tony, thanks for reporting, do I understand correctly that you upgraded, use typescript, and the compiler complains?

Witoso avatar Apr 07 '23 06:04 Witoso

I think we should make this body collection DOM container public. I remember several use cases where developers need to access it:

  • to add a class there, 
  • to move the entire container e.g. to the same parent w/ overflow: hidden that hosts the editing root so all the pieces of the UIs get cut in the same way,
  • to move the entire container to a different CSS context (e.g. to inherit styles), right now it's always straight in body > .

oleq avatar Apr 14 '23 11:04 oleq

I think we should make this body collection DOM container public.

Tracking in: https://github.com/ckeditor/ckeditor5/issues/13928.

Witoso avatar Apr 20 '23 06:04 Witoso

I think we should make this body collection DOM container public.

Tracking in: #13928.

Merged to master.

arkflpc avatar May 26 '23 09:05 arkflpc

Other use case: styling the elements in the BodyCollection (e.g. balloons) differently for a few editors at the same webpage. Encountered while trying to style PoweredBy logo in two various manners (for the normal theme and the dark one). The workaround was to add the custom class to the BodyCollection._bodyCollectionContainer using the DOM API. With this issue solved, it would be simply the matter of passing the classname in config.

Dumluregn avatar May 29 '23 12:05 Dumluregn

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may still be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

CKEditorBot avatar May 29 '24 03:05 CKEditorBot

We've closed your issue due to inactivity. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).

CKEditorBot avatar Jun 29 '24 03:06 CKEditorBot

@ckeditor I'm fine keeping this being open if others want it. We use @oleq's snippet from https://github.com/ckeditor/ckeditor5/issues/9539#issuecomment-833491911.

tony avatar Jun 29 '24 14:06 tony