Overview

Not categorizing as *bug* because this, like many other things XB, is functionality that hasn't been built yet. Nothing has been broken, etc.

I assume we want this? If the wysiwyg is provided in node/edit, we oughta provide it in the XB equivalent.

Proposed resolution

  • CKeditor 5 appears in fields it is expected to appear in
  • The contents can be updated with CKEditor 5 formatting. We do not need to evaluate nuances of the full feature set of CKEditor 5, we just need to confirm something can be formatted for now
  • Dialogs triggered from CKEditor 5 do not have to work yet - this is covered in a different issue
  • If some toolbar items are not visible or the sticky positioning isn't quite right this is fine , it will be covered in a different issue

Getting the basic functionality of CKEditor 5 in now unlocks a bunch of additional needed work. The minutiae can be addressed in later issues.

User interface changes

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

bnjmnm created an issue. See original summary.

wim leers’s picture

Title: CKeditor not loading on formatted text fields in the content entity + component instance forms » CKEditor not loading on formatted text fields in the content entity + component instance forms
Related issues: +#3512433: Provide visibility into which (core) field types (74%), field type props (63%) can be mapped into Content Type Templates vs not, and which field widgets (36%) are supported

💯 — and this was surfaced in #3512433-6: Provide visibility into which (core) field types (74%), field type props (63%) can be mapped into Content Type Templates vs not, and which field widgets (36%) are supported earlier today (for component instance forms specifically):

…, text_textarea, text_textarea_with_summary, text_textfield, …

🤓

Note that this is a special beast, see \Drupal\editor\Element::preRenderTextFormat():

    // \Drupal\filter\Element\TextFormat::processFormat() copies properties to
    // the expanded 'value' to the child element, including the #pre_render
    // property. Skip this text format widget, if it contains no 'format'.
    if (!isset($element['format'])) {
      return $element;
    }

and

    foreach ($editors as $key => $editor) {
      $definition = $this->pluginManager->getDefinition($editor->getEditor());
      if (!in_array($element['#base_type'], $definition['supported_element_types'])) {
        unset($editors[$key]);
      }
    }

— IOW the Text Editor (editor) module alters the text field type's widget's form render array representation 😅

bnjmnm’s picture

StatusFileSize
new46.4 KB

MR I just opened has logic that gets CKEditor5 to appear. It's not ready for prime-time yet but the underlying issue has been identified and there is a solution.

wim leers’s picture

Nice first step! Does CKEditor 5 itself also work?

wim leers’s picture

Title: CKEditor not loading on formatted text fields in the content entity + component instance forms » CKEditor 5 not loading on formatted text Field Widgets in the content entity + component instance forms
bnjmnm’s picture

Does CKEditor 5 itself also work?

The CKEditor 5 editor works - you can format the text using the various buttons like one might expect.

Do the contents of the edited-by-CKEdtor5 field get saved? That part does not appear to work. Unless it proves reasonably simple, that can be tackled in a separate issue once this lands.

bnjmnm’s picture

My next step was to see if changing editors worked, and ran into a situation that will likely require nontrivial refactoring. The underlying cause may cause problems for more than just this use case.

Radix <Select> does result in a <select> tag in the UI. It is instead a button that triggers the appearance of selectable options.

Drupal.behaviors.editor very much expects a <select>, in some cases not adding the listeners unless that is the element type.

 if (editor.tagName === 'SELECT') {
          $this.on('change.editorAttach', { field }, onTextFormatChange);
        }

The Radix select does have a hidden <select>, which is not exposed as a ref in @radix-ui/themes, but looks like it miiight be accessible if we access it more directly instead of the theme. I did some exploring and this doesn't look like something that can be done quickly The only ref we can access via Radix is the <button> offered by the UI. Because this ref is available, it's possible we can use it as the basis for a querySelector to get the <select> but we'd need to see if that indirect-reffing is stable. If that can't work the only alternative that comes to mind is either patching Radix or using something other than Radix for this particular element. Other solutions might occur to me in time.

bnjmnm’s picture

Got this working by querying from the available ref

const hiddenSelect =
      selectRef.current.parentElement?.querySelector<HTMLSelectElement>(
        'select[aria-hidden]',
      );

Doesn't feel the most solid to me, but maybe it's OK. So far seems to work.

wim leers’s picture

@bnjmnm in #2 I expanded the scope to + component instance forms. Back then, I didn't yet understand that the content entity form needs a very different treatment! 🫣

In HEAD, you literally can't get such a widget to show up in the component instance form. But with #3467959-51: SDC and code component props should be able to receive HTML, editable in formatted text fields+widgets, you can:

Would you prefer:

  1. to narrow the scope here back to only the content entity form?
  2. to tackle both here?

The choice is yours :)

wim leers’s picture

Title: CKEditor 5 not loading on formatted text Field Widgets in the content entity + component instance forms » CKEditor 5 not loading on formatted text Field Widgets in the content entity form

Discussed #11 with @bnjmnm — let's first land this one, because it's so close!

wim leers’s picture

wim leers’s picture

Component: Redux-integrated field widgets » Semi-Coupled theme engine
Assigned: Unassigned » bnjmnm
Issue summary: View changes
Status: Active » Needs work
StatusFileSize
new127.68 KB
new20.44 KB
new27.94 KB
new1.24 MB
new1.95 MB

Changing issue queue component because this really only affects the Semi-Coupled Theme Engine ui/src/components/form/components/drupal/*.tsx!


To help get this over the line, I tested this MR manually.

The essentials work, but not everything:

  1. the dialog to switch to a different text format renders incorrectly and is inaccessible:
  2. not all CKEditor 5 toolbar buttons are accessible. Visible:

    Actual (screenshot of /admin/config/content/formats/manage/basic_html):

  3. if I move the "image" button to the accessible part of the toolbar, it works:
  4. if I add the media library functionality and move it to the accessible part of the toolbar, it almost works — styling looks off, and I cannot type an alternative text so I cannot actually upload new images into the media library:

@bnjmnm: Thoughts on which of these to tackle here vs deferring to follow-ups?

nagwani’s picture

Issue tags: +sprint
bnjmnm’s picture

wim leers’s picture

Status: Needs review » Reviewed & tested by the community

Progress! Let's get this in! 🚀

  • wim leers committed 3d909e46 on 0.x authored by bnjmnm
    Issue #3512867 by bnjmnm, wim leers: CKEditor 5 not loading on formatted...
wim leers’s picture

Status: Reviewed & tested by the community » Fixed
larowlan’s picture

Status: Fixed » Needs work

Opening a follow up for a few things spotted in posthumous review

larowlan’s picture

Status: Needs work » Needs review

Opened a new MR with the stuff I had in an in progress review that I finished off this morning after it had already been merged - should have finished it yesterday - sorry!

wim leers’s picture

Assigned: Unassigned » bnjmnm

🙏 Could you review Lee’s additions here, Ben?

  • bnjmnm committed 0dcb1e5c on 0.x authored by larowlan
    Issue #3512867 by larowlan: CKEditor 5 not loading on formatted text...
bnjmnm’s picture

Assigned: bnjmnm » Unassigned
Status: Needs review » Fixed

The followup MR was straightforward and welcome, added.

bnjmnm’s picture

bnjmnm’s picture

Issue summary: View changes
wim leers’s picture

wim leers’s picture

nagwani’s picture

Issue tags: -sprint

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.