CKEditor5 Drupalbreaks update

Created on 20 October 2023, about 1 year ago

Problem/Motivation

Update for CKEditor 5 Drupalbreaks as mentioned by dsnopek in issue 3296752 πŸ“Œ Automated Drupal 10 compatibility fixes Fixed

Creating a new issue fork, cleanup and doing a MR

API changes

- adding Filter.php to ensure gets rendered as intended
- updating codebase to CKEDitor 5 MVC pattern

πŸ“Œ Task
Status

Active

Version

2.0

Component

Code

Created by

πŸ‡©πŸ‡ͺGermany Istari

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @Istari
  • πŸ‡©πŸ‡ͺGermany Istari

    Having a problem with pushing my code.
    The error message says:
    "remote: You are not allowed to push code to this project.
    fatal: unable to access 'https://git.drupalcode.org/project/ckeditor_drupalbreaks.git/': The requested URL returned error: 403"
    when trying to push my branch into 2.0.x.

    I created a new custom access token and use those information to log in, but it keeps denying the access..
    Any ideas?

  • πŸ‡ΊπŸ‡ΈUnited States dsnopek USA

    It looks like you're trying to push to the main project repository, rather than the issue fork. Try changing your remote to git@git.drupal.org:issue/ckeditor_drupalbreaks-3395704.git - the git commands would be something like:

    git remote set-url origin git@git.drupal.org:issue/ckeditor_drupalbreaks-3395704.git
    

    But if all else fails, you can always attach a patch the old-fashioned way :-)

  • πŸ‡©πŸ‡ͺGermany Istari

    thx @dsnopek for your help.
    I pushed the changes. If anything is unclear or need to be done just tell me :)

  • @dsnopek opened merge request.
  • Status changed to Needs work about 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States dsnopek USA

    @Mysdiir Thanks!

    So that it'd be easier to put review comments on the diff I made an MR and added quite a few notes:

    https://git.drupalcode.org/project/ckeditor_drupalbreaks/-/merge_requests/3

    Please let me know if you're interested in making those changes. Otherwise, I'd be happy to take this over and finish it up :-)

  • πŸ‡©πŸ‡ͺGermany Istari

    Sure if you want to change it feel free to do it :)
    When it's all finished, I'll look up the changes and see what I can learn from it :)

  • πŸ‡©πŸ‡ͺGermany Istari

    made suggessted changes with removing package.json, package-lock.json, yarn.lock and webpack.config (for generating a minified version).

    So you should have access to those files

  • Status changed to Needs review about 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States dsnopek USA

    Thanks!

    I've updated the code on the GitLab issue fork such that it's now working in my testing :-)

  • πŸ‡©πŸ‡ͺGermany Istari

    If needed I can test it on local setup too, so we have a double confirmation that it works

  • πŸ‡©πŸ‡ͺGermany drupatz

    @dsnopek Hi, using the element <drupalbreaks> causes the cke adding &nbsp; and <p> tags, resulting in <p><drupalbreaks>&nbsp;</drupalbreaks></p> -- in my point of view this isn't a sober solution. Therefore i did that via the hr element: <hr class="cke-drupal-break">, where this doesn't happen. Of course, this also isn't a sober solution.

    When using <drupalbreaks>, maybe there is a possibility in drupalbreaksEditing.js to tell cke to not add anything?

  • πŸ‡©πŸ‡ͺGermany Istari

    @drupatz The problem with not adding any HTML tag values in drupalbreaksEditing.js is, that this file provides the logic for DOM manipulation between CKEditor and Drupal, said in MVC pattern: the CKEditor upcast and downcast pipeline are representative for view layer and model layer. This is where adding the <drupalbreaks> tag insertion happens and gets converted to Drupal.

    The basic logic behind my module modification is that CKEditor manipulates the DOM with insertion of the HTML tag.
    This tag is fetched by DrupalbreaksFilter.php and swapps <drupalbreaks> to <!--break--> which Drupal magic interprets as break between content and teaser.

    But I agree with you that the additional <p> is unnessesary.

    I'd appreciate a feedback if the merged changes by @dsnopek are working as intended.

  • πŸ‡ΊπŸ‡ΈUnited States dsnopek USA

    @drupatz:

    Hi, using the element <drupalbreaks> causes the cke adding &nbsp; and <p> tags, resulting in <p><drupalbreaks>&nbsp;</drupalbreaks></p> -- in my point of view this isn't a sober solution.

    As @Mysdiir mentions, if you enable the Drupalbreaks filter on your text format, it will convert the <drupalbreaks>&nbsp;</drupalbreaks></p> to <!--break-->.

    But I agree with you that the additional <p> is unnessesary.

    I've just pushed a commit that will get rid of the extra <p> too.

    I'm thinking of merging this into a new branch (perhaps 2.1.x or 3.0.x -- I'm not sure yet) so that it's easier to pull into a site and test. We're not fully committed to this approach until we make a proper release. :-)

  • πŸ‡ΊπŸ‡ΈUnited States dsnopek USA
  • πŸ‡ΈπŸ‡ͺSweden AndKar

    Thanks for this update! It is working fine!

  • πŸ‡©πŸ‡ͺGermany tobiasb Berlin

    Missing is a upgrade path (Need to manually enable button again) and existing <!--break--> are gone, when open content with CKEditor5.

  • πŸ‡ΊπŸ‡ΈUnited States codesmith San Francisco

    This worked pretty well for me on Drupal 10/CK editor 5. I had to re-add the button to the editor and enable the "Process the output of the Drupalbreaks CKEditor plugin" filter. Existing content that had a teaser break worked fine on the front end. When editing however, the double red teaser line isn't shown.

    <!--break-->

    is in the code but is not highlighted - only the new " " is highlighted.

    So while this does work, I'd agree with #17 that there needs to be an upgrade to convert "

    <!--break-->

    " in content to " "

  • πŸ‡ΊπŸ‡ΈUnited States ladybug_3777

    I had the same experience as #18. Had to re-add the button and enable the filter (I missed the filter step my first time through).

    I also noticed that when editing existing content the red highlight was not displayed, but re-saving the content worked fine and did not alter functionality as long as the underlying

    <!--break-->

    stayed intact.

    Adding new breaks with the new button worked fine.

  • πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

    @dsnopek any plans to finish this and make the module D11 compatible? Or is the module unmaintained?
    Thanks! :)

Production build 0.71.5 2024