From a30dc37e3c6ee086c13bebe1a8efafdefa51c320 Mon Sep 17 00:00:00 2001 From: Mateusz Baginski Date: Tue, 22 Oct 2024 07:39:36 +0200 Subject: [PATCH] Restore next highlighted item after serach & replace operation. --- .../src/findandreplaceediting.ts | 24 +++++++++++++++++-- .../src/findandreplacestate.ts | 20 ++++++++++++---- .../tests/ui/findandreplaceformview.js | 2 +- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/packages/ckeditor5-find-and-replace/src/findandreplaceediting.ts b/packages/ckeditor5-find-and-replace/src/findandreplaceediting.ts index dbc061be2a5..f2038e06f5a 100644 --- a/packages/ckeditor5-find-and-replace/src/findandreplaceediting.ts +++ b/packages/ckeditor5-find-and-replace/src/findandreplaceediting.ts @@ -21,7 +21,7 @@ import ReplaceCommand from './replacecommand.js'; import ReplaceAllCommand from './replaceallcommand.js'; import FindNextCommand from './findnextcommand.js'; import FindPreviousCommand from './findpreviouscommand.js'; -import FindAndReplaceState, { type FindCallback } from './findandreplacestate.js'; +import FindAndReplaceState, { sortSearchResultsByMarkerPositions, type FindCallback } from './findandreplacestate.js'; import FindAndReplaceUtils from './findandreplaceutils.js'; import type { ResultType } from './findandreplace.js'; @@ -216,6 +216,20 @@ export default class FindAndReplaceEditing extends Plugin { const model = this.editor.model; const { results } = this.state!; + // We save highlight offset here, as the information about the highlighted result will be lost after the changes. + // + // It happens because result list is partially regenerated if the result is removed from the paragraph. + // Partially means that all sibling result items that are placed in the same paragraph are removed and added again, + // which causes the highlighted result to be malformed (usually it's set to first but it's not guaranteed). + // + // Keep in mind that the highlighted offset is indexed from 1, as it's displayed to the user. It's why we subtract 1 here. + // + // More info: https://github.com/ckeditor/ckeditor5/issues/16648 + const oldHighlightOffset = Math.max( + this.state!.highlightedOffset - 1, + 0 + ); + const changes = model.document.differ.getChanges() as Array>; const changedMarkers = model.document.differ.getChangedMarkers(); @@ -287,9 +301,15 @@ export default class FindAndReplaceEditing extends Plugin { } } ); + // If currently highlighted result was removed, select the next one after removed one. + // It stays at the same offset as previous one. if ( !this.state!.highlightedResult && changedSearchResults.length ) { + // Use modulo to avoid out of bounds error, just wrap around. + const sortedResults = sortSearchResultsByMarkerPositions( this.state!.results ); + const restoreNextResult = sortedResults.get( oldHighlightOffset % sortedResults.length ); + // If there are found phrases but none is selected, select the first one. - this.state!.highlightedResult = changedSearchResults[ 0 ]; + this.state!.highlightedResult = restoreNextResult; } else { // If there is already highlight item then refresh highlight offset after appending new items. this.state!.refreshHighlightOffset(); diff --git a/packages/ckeditor5-find-and-replace/src/findandreplacestate.ts b/packages/ckeditor5-find-and-replace/src/findandreplacestate.ts index e2f1adbeb5b..6fa813c3512 100644 --- a/packages/ckeditor5-find-and-replace/src/findandreplacestate.ts +++ b/packages/ckeditor5-find-and-replace/src/findandreplacestate.ts @@ -151,18 +151,30 @@ export default class FindAndReplaceState extends /* #__PURE__ */ ObservableMixin */ public refreshHighlightOffset(): void { const { highlightedResult, results } = this; - const sortMapping = { before: -1, same: 0, after: 1, different: 1 }; if ( highlightedResult ) { - this.highlightedOffset = Array.from( results ) - .sort( ( a, b ) => sortMapping[ a.marker!.getStart().compareWith( b.marker!.getStart() ) ] ) - .indexOf( highlightedResult ) + 1; + this.highlightedOffset = sortSearchResultsByMarkerPositions( results ).getIndex( highlightedResult ) + 1; } else { this.highlightedOffset = 0; } } } +/** + * Sorts search results by marker positions. Make sure that the results are sorted in the same order as they appear in the document + * to avoid issues with the `find next` command. Apparently, the order of the results in the state might be different than the order + * of the markers in the model. + */ +export function sortSearchResultsByMarkerPositions( results: Collection ): Collection { + const sortMapping = { before: -1, same: 0, after: 1, different: 1 }; + + return new Collection( + Array + .from( results ) + .sort( ( a, b ) => sortMapping[ a.marker!.getStart().compareWith( b.marker!.getStart() ) ] ) + ); +} + /** * The callback function used to find matches in the document. */ diff --git a/packages/ckeditor5-find-and-replace/tests/ui/findandreplaceformview.js b/packages/ckeditor5-find-and-replace/tests/ui/findandreplaceformview.js index 750c62ca041..5e3b86b15e9 100644 --- a/packages/ckeditor5-find-and-replace/tests/ui/findandreplaceformview.js +++ b/packages/ckeditor5-find-and-replace/tests/ui/findandreplaceformview.js @@ -1286,7 +1286,7 @@ describe( 'FindAndReplaceFormView', () => { expect( matchCounterElement.textContent ).to.equal( '2 of 3' ); replaceButton.fire( 'execute' ); - expect( matchCounterElement.textContent ).to.equal( '1 of 2' ); + expect( matchCounterElement.textContent ).to.equal( '2 of 2' ); replaceButton.fire( 'execute' ); expect( matchCounterElement.textContent ).to.equal( '1 of 1' );