Skip to content

Bump CKEditor to 48.0#22837

Merged
oliverguenther merged 4 commits into
devfrom
bump/ckeditor
Apr 27, 2026
Merged

Bump CKEditor to 48.0#22837
oliverguenther merged 4 commits into
devfrom
bump/ckeditor

Conversation

@oliverguenther
Copy link
Copy Markdown
Member

@oliverguenther oliverguenther commented Apr 20, 2026

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
await import(/* webpackChunkName: "ckeditor" */ 'core-vendor/ckeditor/ckeditor');
const loadEditorScript = import(/* webpackChunkName: "ckeditor" */ 'core-vendor/ckeditor/ckeditor');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick, but /* webpackChunkName: "ckeditor" */ can be removed now. esbuild ignores.

*/
private async load():Promise<void> {
// untyped module cannot be dynamically imported
// untyped modules cannot be dynamically imported
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, this typing can/should be fixed eventually: https://community.openproject.org/wp/72830

(I started work on opf/commonmark-ckeditor-build#109 a while back)

@myabc
Copy link
Copy Markdown
Contributor

myabc commented Apr 23, 2026

@oliverguenther it looks like the text size and typeface is different (16px Helvetica vs 14px System), at least when editing a description on the work packages full view:

Before After
Screenshot 2026-04-23 at 17 08 02 Screenshot 2026-04-23 at 17 06 42

Also noticeable when inplace editing project descriptions on the Overview page: (cc @HDinger)

image

Not sure if this is intentional or desired?

@myabc
Copy link
Copy Markdown
Contributor

myabc commented Apr 23, 2026

Not sure if this is intentional or desired?

Or because we're bundling styles that we weren't before - on with a different load order?

@myabc myabc self-requested a review April 23, 2026 15:31
Copy link
Copy Markdown
Contributor

@myabc myabc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from the text style changes, this seems to work as expected. 👍🏻 I manually tested the CodeMirror integration and that works well.

(with regards to CodeMirror, I began an attempt at upgrading to CodeMirror 6 last year - #19566. The API and internal event model is substantially different, but I did get the basics working)

Copy link
Copy Markdown
Contributor

@as-op as-op left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
I could not find any regression; all works as expected, including the PDF-related features like page-break.

Like Alex mentioned, the default CSS changed, so the font size and font family are different now.

The changes come from

--ck-content-font-family: Helvetica, Arial, Tahoma, Verdana, Sans-Serif;

and
--ck-content-font-size: medium;

f I change them to

  --ck-content-font-family: inherit;
  --ck-content-font-size: inherit;

The old values seem to be used again.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 27, 2026

Deploying openproject with PullPreview

Field Value
Latest commit 93034bc
Job deploy
Status ✅ Deploy successful
Preview URL https://pr-22837-ckeditor-ip-178-105-21-67.my.opf.run:443

View logs

@oliverguenther oliverguenther merged commit eeff1ca into dev Apr 27, 2026
19 of 20 checks passed
@oliverguenther oliverguenther deleted the bump/ckeditor branch April 27, 2026 12:49
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 27, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants