-
Notifications
You must be signed in to change notification settings - Fork 7
[#69553] Remove jQuery from CKEditor build #107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
97eb707
865c9de
ab1b098
6575b57
0497dc1
c6e294d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| # Changelog | ||
|
|
||
| ## [Unreleased] | ||
|
|
||
| ### Changed | ||
|
|
||
| - Removed jQuery dependency from the CKEditor build. All jQuery usage has been replaced with vanilla JavaScript equivalents: | ||
| - `jQuery.each()` replaced with native `Array.forEach()` | ||
| - `jQuery.getJSON()` replaced with `fetch()` API | ||
| - `jQuery.ajax()` replaced with `fetch()` API | ||
| - jQuery DOM manipulation replaced with native DOM APIs (`document.createElement()`, `element.parentElement`, `element.style`, etc.) | ||
|
|
||
| ### Migration Notes | ||
|
|
||
| This library no longer uses jQuery internally. However, downstream consumers (such as OpenProject) should keep the jQuery global available for other parts of the application until they are ready to remove it. This change only affects the internal implementation of this library and should not require changes in consuming applications. | ||
|
|
||
| For more context, see the related OpenProject pull request: https://github.com/opf/openproject/pull/19429 |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -30,15 +30,20 @@ export default class OPPreviewPlugin extends Plugin { | |||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| let showPreview = function(preview) { | ||||||||||||||
| let $reference = jQuery(editor.ui.getEditableElement()).parent(); | ||||||||||||||
| let $previewWrapper = jQuery('<div class="ck-editor__preview op-uc-container"></div>'); | ||||||||||||||
| $reference.siblings('.ck-editor__preview').remove(); | ||||||||||||||
| const editableElement = editor.ui.getEditableElement(); | ||||||||||||||
| const reference = editableElement.parentElement; | ||||||||||||||
| const previewWrapper = document.createElement('div'); | ||||||||||||||
| previewWrapper.className = 'ck-editor__preview op-uc-container'; | ||||||||||||||
|
|
||||||||||||||
| // Remove existing preview elements | ||||||||||||||
| const existingPreviews = reference.parentElement.querySelectorAll('.ck-editor__preview'); | ||||||||||||||
myabc marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||
| existingPreviews.forEach(el => el.remove()); | ||||||||||||||
|
|
||||||||||||||
| const previewService = getOPService(editor, 'ckEditorPreview'); | ||||||||||||||
| unregisterPreview = previewService.render($previewWrapper[0], preview); | ||||||||||||||
| unregisterPreview = previewService.render(previewWrapper, preview); | ||||||||||||||
|
|
||||||||||||||
| $reference.hide(); | ||||||||||||||
| $reference.after($previewWrapper); | ||||||||||||||
| reference.style.display = 'none'; | ||||||||||||||
| reference.parentElement.insertBefore(previewWrapper, reference.nextSibling); | ||||||||||||||
|
|
||||||||||||||
| disableItems(editor, view); | ||||||||||||||
| }; | ||||||||||||||
|
|
@@ -47,22 +52,38 @@ export default class OPPreviewPlugin extends Plugin { | |||||||||||||
| let link = getOPPreviewContext(editor); | ||||||||||||||
| let url = getOPPath(editor).api.v3.previewMarkup(link); | ||||||||||||||
|
|
||||||||||||||
| jQuery | ||||||||||||||
| .ajax({ | ||||||||||||||
| data: editor.getData(), | ||||||||||||||
| url: url, | ||||||||||||||
| response_type: 'text', | ||||||||||||||
| contentType: 'text/plain; charset=UTF-8', | ||||||||||||||
| method: 'POST', | ||||||||||||||
| }).done(showPreview); | ||||||||||||||
| fetch(url, { | ||||||||||||||
| method: 'POST', | ||||||||||||||
| headers: { | ||||||||||||||
| 'Content-Type': 'text/plain; charset=UTF-8' | ||||||||||||||
| }, | ||||||||||||||
| body: editor.getData() | ||||||||||||||
| }) | ||||||||||||||
| .then(response => { | ||||||||||||||
| if (!response.ok) { | ||||||||||||||
| throw new Error(`HTTP error! status: ${response.status}`); | ||||||||||||||
| } | ||||||||||||||
| return response.text(); | ||||||||||||||
| }) | ||||||||||||||
| .then(showPreview) | ||||||||||||||
| .catch(error => { | ||||||||||||||
| console.error('Error fetching preview:', error); | ||||||||||||||
myabc marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
| }); | ||||||||||||||
| }; | ||||||||||||||
|
|
||||||||||||||
| let disablePreviewing = function() { | ||||||||||||||
| let $mainEditor = jQuery(editor.ui.getEditableElement()).parent(); | ||||||||||||||
| const editableElement = editor.ui.getEditableElement(); | ||||||||||||||
| const mainEditor = editableElement.parentElement; | ||||||||||||||
|
||||||||||||||
| const mainEditor = editableElement.parentElement; | |
| const mainEditor = editableElement && editableElement.parentElement; | |
| if (!mainEditor || !mainEditor.parentElement) { | |
| console.error('Cannot disable preview: invalid editor structure'); | |
| return; | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot apply suggestion, but use optional chaining
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 0497dc1 - added optional chaining (?.) and early return guard with error logging as requested
myabc marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -35,12 +35,17 @@ export default class OPSourceCodePlugin extends Plugin { | |||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| let showSource = function(_preview) { | ||||||||||||||
| let $reference = jQuery(editor.ui.getEditableElement()).parent(); | ||||||||||||||
| let $sourceWrapper = jQuery('<div class="ck-editor__source"></div>'); | ||||||||||||||
| $reference.siblings('.ck-editor__source').remove(); | ||||||||||||||
|
|
||||||||||||||
| $reference.hide(); | ||||||||||||||
| $reference.after($sourceWrapper); | ||||||||||||||
| const editableElement = editor.ui.getEditableElement(); | ||||||||||||||
| const reference = editableElement.parentElement; | ||||||||||||||
|
||||||||||||||
| const sourceWrapper = document.createElement('div'); | ||||||||||||||
| sourceWrapper.className = 'ck-editor__source'; | ||||||||||||||
|
|
||||||||||||||
| // Remove existing source elements | ||||||||||||||
| const existingSources = reference.parentElement.querySelectorAll('.ck-editor__source'); | ||||||||||||||
|
||||||||||||||
| existingSources.forEach(el => el.remove()); | ||||||||||||||
|
|
||||||||||||||
| reference.style.display = 'none'; | ||||||||||||||
| reference.parentElement.insertBefore(sourceWrapper, reference.nextSibling); | ||||||||||||||
|
|
||||||||||||||
| disableItems(editor, view); | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -56,12 +61,16 @@ export default class OPSourceCodePlugin extends Plugin { | |||||||||||||
| }; | ||||||||||||||
|
|
||||||||||||||
| let hideSource = function() { | ||||||||||||||
| let $mainEditor = jQuery(editor.ui.getEditableElement()).parent(); | ||||||||||||||
| const editableElement = editor.ui.getEditableElement(); | ||||||||||||||
| const mainEditor = editableElement.parentElement; | ||||||||||||||
|
||||||||||||||
| const mainEditor = editableElement.parentElement; | |
| const mainEditor = editableElement.parentElement; | |
| if (!mainEditor || !mainEditor.parentElement) { | |
| console.error('Cannot hide source: invalid editor structure'); | |
| return; | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot apply suggestion, but use optional chaining
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 0497dc1 - added optional chaining (?.) and early return guard with error logging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential null reference error if
editableElement.parentElementis null. The code immediately accessesreference.parentElementon lines 39 and 46 without checking ifreference(which iseditableElement.parentElement) is null first.Consider adding a null check:
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot apply suggestion, but use optional chaining
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 0497dc1 - added optional chaining (
?.) and early return guard with error logging