Skip to content

Commit 954b7d4

Browse files
committed
MC-4014: PageBuilder Performance Is Bad With Minimal Content
- Improve comment regarding null’ing the wysiwyg member - Ensure duplicate text nodes with the same value are supported by our selecting logic
1 parent 79bb849 commit 954b7d4

File tree

2 files changed

+41
-22
lines changed
  • app/code/Magento/PageBuilder/view/adminhtml/web

2 files changed

+41
-22
lines changed

app/code/Magento/PageBuilder/view/adminhtml/web/js/content-type/slide/preview.js

Lines changed: 18 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

app/code/Magento/PageBuilder/view/adminhtml/web/ts/js/content-type/slide/preview.ts

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,11 @@ export default class Preview extends BasePreview {
5757
this.element = element;
5858
element.id = this.parent.id + "-editor";
5959

60-
// This function is called whenever a render happens, meaning our WYSIWYG instance is destroyed
60+
/**
61+
* afterRenderWysiwyg is called whenever Knockout causes a DOM re-render. This occurs frequently within Slider
62+
* due to Slick's inability to perform a refresh with Knockout managing the DOM. Due to this the original
63+
* WYSIWYG instance will be detached from this slide and we need to re-initialize on click.
64+
*/
6165
this.wysiwyg = null;
6266
}
6367

@@ -355,6 +359,8 @@ export default class Preview extends BasePreview {
355359
const selection = window.getSelection();
356360
if (selection.getRangeAt && selection.rangeCount) {
357361
const range = selection.getRangeAt(0).cloneRange();
362+
$(range.startContainer.parentNode).attr("data-startContainer", "true");
363+
$(range.endContainer.parentNode).attr("data-endContainer", "true");
358364
return {
359365
startContainer: range.startContainer,
360366
startOffset: range.startOffset,
@@ -375,10 +381,20 @@ export default class Preview extends BasePreview {
375381
private restoreSelection(element: HTMLElement, selection: Selection) {
376382
if (window.getSelection) {
377383
// Find the original container that had the selection
378-
const startContainer: HTMLElement = this.findTextNode(element, selection.startContainer.nodeValue);
384+
const startContainerParent = $(element).find("[data-startContainer]");
385+
startContainerParent.removeAttr("data-startContainer");
386+
const startContainer: HTMLElement = this.findTextNode(
387+
startContainerParent,
388+
selection.startContainer.nodeValue,
389+
);
390+
const endContainerParent = $(element).find("[data-endContainer]");
391+
endContainerParent.removeAttr("data-endContainer");
379392
let endContainer: HTMLElement = startContainer;
380393
if (selection.endContainer.nodeValue !== selection.startContainer.nodeValue) {
381-
endContainer = this.findTextNode(element, selection.endContainer.nodeValue);
394+
endContainer = this.findTextNode(
395+
endContainerParent,
396+
selection.endContainer.nodeValue,
397+
);
382398
}
383399

384400
if (startContainer && endContainer) {
@@ -400,15 +416,11 @@ export default class Preview extends BasePreview {
400416
* @param {string} text
401417
* @returns {HTMLElement}
402418
*/
403-
private findTextNode(element: HTMLElement, text: string): HTMLElement {
419+
private findTextNode(element: JQuery, text: string): HTMLElement {
404420
if (text && text.trim().length > 0) {
405-
const textSearch = $(element).find(":contains(\"" + text.trim() + "\")");
406-
if (textSearch.length > 0) {
407-
// Search for the #text node within the element for the new range
408-
return textSearch.last().contents().filter(function() {
409-
return this.nodeType === Node.TEXT_NODE && text === this.nodeValue;
410-
})[0];
411-
}
421+
return element.contents().filter(function() {
422+
return this.nodeType === Node.TEXT_NODE && text === this.nodeValue;
423+
})[0];
412424
}
413425
}
414426
}

0 commit comments

Comments
 (0)