Skip to content

Commit

Permalink
[BUGFIX] Improve performance of "View Upgrade Documentation"
Browse files Browse the repository at this point in the history
This patch greatly improves the performance of the
"View Upgrade Documentation" module in the Install Tool.

The following optimizations are done:

* The tag filter is initialized only once now to avoid double-filtering
  documents
* Document tags are now rendered directly in Fluid, removing one
  expensive document manipulation task
* Composition of the tag list for the tag filter is sped up by a factor
  of 1.5 - 2 by using Set() to filter most duplicates
* When emptying a search, internal upgrade doc classes are removed once
  all panels are hidden
* When filtering, matching version panels are expanded with a delay of
  20ms, relaxing re-layouts and repaints in the browser


Resolves: #98405
Releases: main
Change-Id: I2e6b68c009652a23d088b27714b925b0e809aaa0
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/75840
Tested-by: core-ci <typo3@b13.com>
Tested-by: Susanne Moog <look@susi.dev>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
  • Loading branch information
andreaskienast committed Sep 22, 2022
1 parent 22082c4 commit 308b6e5
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 64 deletions.
104 changes: 42 additions & 62 deletions Build/Sources/TypeScript/install/module/upgrade/upgrade-docs.ts
Expand Up @@ -36,20 +36,6 @@ class UpgradeDocs extends AbstractInteractableModule {
private chosenField: JQuery;
private fulltextSearchField: JQuery;

private static trimExplodeAndUnique(delimiter: string, string: string): Array<string> {
const result: Array<string> = [];
const items = string.split(delimiter);
for (let i = 0; i < items.length; i++) {
const item = items[i].trim();
if (item.length > 0) {
if ($.inArray(item, result) === -1) {
result.push(item);
}
}
}
return result;
}

public initialize(currentModal: JQuery): void {
this.currentModal = currentModal;
const isInIframe = (window.location !== window.parent.location);
Expand Down Expand Up @@ -81,9 +67,7 @@ class UpgradeDocs extends AbstractInteractableModule {

private getContent(): void {
const modalContent = this.getModalBody();
modalContent.on('show.bs.collapse', this.selectorUpgradeDoc, (e: JQueryEventObject): void => {
this.renderTags($(e.currentTarget));
});

(new AjaxRequest(Router.getUrl('upgradeDocsGetContent')))
.get({cache: 'no-cache'})
.then(
Expand All @@ -104,7 +88,7 @@ class UpgradeDocs extends AbstractInteractableModule {
}

private loadChangelogs(): void {
const promises: Array<Promise<any>> = [];
const promises: Array<Promise<AjaxRequest>> = [];
const modalContent = this.getModalBody();
this.findInModal(this.selectorChangeLogsForVersionContainer).each((index: number, el: any): void => {
const request = (new AjaxRequest(Router.getUrl('upgradeDocsGetChangelogForVersion')))
Expand Down Expand Up @@ -153,8 +137,6 @@ class UpgradeDocs extends AbstractInteractableModule {
});
searchInput.focus();

this.initializeChosenSelector();

new DebounceEvent('keyup', (): void => {
this.combinedFilterSearch();
}).bindTo(searchInput);
Expand All @@ -181,59 +163,64 @@ class UpgradeDocs extends AbstractInteractableModule {
}

/**
* Appends tags to the chosen selector
* Appends tags to the chosen selector in multiple steps:
*
* 1. create a flat CSV of tags
* 2. create a Set() with those tags, automatically filtering duplicates
* 3. reduce remaining duplicates due to the case sensitivity behavior of Set(), while keeping the original case of
* the first item of a set of dupes
* 4. sort tags
*/
private appendItemsToChosenSelector(): void {
let tagString = '';
$(this.findInModal(this.selectorUpgradeDoc)).each((index: number, element: any): void => {
tagString += $(element).data('item-tags') + ',';
});
const tagArray = UpgradeDocs.trimExplodeAndUnique(',', tagString).sort((a: string, b: string): number => {
let tagSet = new Set(tagString.slice(0, -1).split(','));
const uniqueTags = [...tagSet.values()].reduce((tagList: string[], tag: string): string[] => {
const normalizedTag = tag.toLowerCase();
if (tagList.every(otherElement => otherElement.toLowerCase() !== normalizedTag)) {
tagList.push(tag);
}

return tagList;
}, []).sort((a: string, b: string): number => {
// Sort case-insensitive by name
return a.toLowerCase().localeCompare(b.toLowerCase());
});

this.chosenField.prop('disabled', false);
$.each(tagArray, (i: number, tag: any): void => {
for (let tag of uniqueTags) {
this.chosenField.append($('<option>').text(tag));
});
}
this.chosenField.trigger('chosen:updated');
}

private combinedFilterSearch(): boolean {
private combinedFilterSearch(): void {
const modalContent = this.getModalBody();
const $items = modalContent.find(this.selectorUpgradeDoc);
if (this.chosenField.val().length < 1 && this.fulltextSearchField.val().length < 1) {
this.currentModal.find('.panel-version .panel-collapse.show').collapse('hide');
$items.removeClass('hidden searchhit filterhit');
return false;
const $expandedPanels = this.currentModal.find('.panel-version .panel-collapse.show');
$expandedPanels.one('hidden.bs.collapse', (): void => {
if (this.currentModal.find('.panel-version .panel-collapse.collapsing').length === 0) {
// Bootstrap doesn't offer promises to check whether all panels are collapsed, so we need a helper to do
// something similar
$items.removeClass('searchhit filterhit');
}
});
$expandedPanels.collapse('hide');
return;
}
$items.addClass('hidden').removeClass('searchhit filterhit');

$items.removeClass('searchhit filterhit');

// apply tags
if (this.chosenField.val().length > 0) {
$items
.addClass('hidden')
.removeClass('filterhit');
const orTags: Array<string> = [];
const andTags: Array<string> = [];
$.each(this.chosenField.val(), (index: number, item: any): void => {
const tagFilter = '[data-item-tags*="' + item + '"]';
if (item.includes(':', 1)) {
orTags.push(tagFilter);
} else {
andTags.push(tagFilter);
}
});
const andString = andTags.join('');
const tags = [];
if (orTags.length) {
for (let orTag of orTags) {
tags.push(andString + orTag);
}
} else {
tags.push(andString);
}
const tagSelection = tags.join(',');

const tagSelection = this.chosenField.val().map((tag: string) => '[data-item-tags*="' + tag + '"]').join('');
modalContent.find(tagSelection)
.removeClass('hidden')
.addClass('searchhit filterhit');
Expand All @@ -253,7 +240,12 @@ class UpgradeDocs extends AbstractInteractableModule {
}
});

modalContent.find('.searchhit').closest('.panel-collapse').collapse('show');
modalContent.find('.searchhit').closest('.panel-collapse').each((index: number, item: Element): void => {
// This is a workaround to improve the browser performance as the panels are not expanded at once
window.setTimeout((): void => {
$(item).collapse('show');
}, 20)
});

// Check for empty panels
modalContent.find('.panel-version').each((index: number, element: any): void => {
Expand All @@ -262,18 +254,6 @@ class UpgradeDocs extends AbstractInteractableModule {
$element.find(' > .panel-collapse').collapse('hide');
}
});

return true;
}

private renderTags($upgradeDocumentContainer: any): void {
const $tagContainer = $upgradeDocumentContainer.find('.t3js-tags');
if ($tagContainer.children().length === 0) {
const tags = $upgradeDocumentContainer.data('item-tags').split(',');
tags.forEach((value: string): void => {
$tagContainer.append($('<span />', {'class': 'badge'}).text(value));
});
}
}

/**
Expand Down
Expand Up @@ -145,6 +145,7 @@ public function getListEntry(string $file): array
}
}
$entry['tagList'] = implode(',', $entry['tags']);
$entry['tagArray'] = $entry['tags'];
$entry['content'] = (string)file_get_contents($file);
$entry['parsedContent'] = $this->parseContent($entry['content']);
$entry['file_hash'] = md5($entry['content']);
Expand Down
Expand Up @@ -27,7 +27,11 @@ <h3 class="panel-title">
</h3>
</div>
<div id="collapse{id}" class="panel-collapse collapse" role="tabpanel" aria-labelledby="heading{id}">
<div class="rst-tags t3js-tags"></div>
<div class="rst-tags">
<f:for each="{fileArray.tags}" as="tag">
<span class="badge">{tag}</span>
</f:for>
</div>
<div class="rst-links">
<f:if condition="{fileArray.url.issue}">
<a href="{fileArray.url.issue}" target="_blank" rel="noreferrer" class="nowrap"><core:icon identifier="actions-window-open" /> Open issue</a>
Expand Down

0 comments on commit 308b6e5

Please sign in to comment.