Skip to content

Commit

Permalink
Restore next highlighted item after search & replace operation.
Browse files Browse the repository at this point in the history
  • Loading branch information
Mati365 committed Oct 29, 2024
1 parent 926400f commit cb838d6
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ export default class FindAndReplaceEditing extends Plugin {
this.state!.highlightedResult = changedSearchResults[ 0 ];
} else {
// If there is already highlight item then refresh highlight offset after appending new items.
this.state!.refreshHighlightOffset();
this.state!.refreshHighlightOffset( model );
}
};
}
27 changes: 21 additions & 6 deletions packages/ckeditor5-find-and-replace/src/findandreplacestate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export default class FindAndReplaceState extends /* #__PURE__ */ ObservableMixin
} );

this.on<ObservableChangeEvent<ResultType | null>>( 'change:highlightedResult', ( ) => {
this.refreshHighlightOffset();
this.refreshHighlightOffset( model );
} );
}

Expand Down Expand Up @@ -149,20 +149,35 @@ export default class FindAndReplaceState extends /* #__PURE__ */ ObservableMixin
/**
* Refreshes the highlight result offset based on it's index within the result list.
*/
public refreshHighlightOffset(): void {
public refreshHighlightOffset( model: Model ): 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( model, [ ...results ] ).indexOf( 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( model: Model, results: Array<ResultType> ): Array<ResultType> {
const sortMapping = { before: -1, same: 0, after: 1, different: 1 };

// `compareWith` doesn't play well with multi-root documents, so we need to sort results by root name first
// and then sort them within each root. It prevents "random" order of results when the document has multiple roots.
// See more: https://github.com/ckeditor/ckeditor5/pull/17292#issuecomment-2442084549
return model.document.getRootNames().flatMap( rootName =>
results
.filter( result => result.marker!.getStart().root.rootName === rootName )
.sort( ( a, b ) => sortMapping[ a.marker!.getStart().compareWith( b.marker!.getStart() ) ] )
);
}

/**
* The callback function used to find matches in the document.
*/
Expand Down
29 changes: 29 additions & 0 deletions packages/ckeditor5-find-and-replace/src/replacecommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

import type { ResultType } from './findandreplace.js';
import { sortSearchResultsByMarkerPositions } from './findandreplacestate.js';
import { ReplaceCommandBase } from './replacecommandbase.js';

/**
Expand All @@ -22,6 +23,34 @@ export default class ReplaceCommand extends ReplaceCommandBase {
* @fires execute
*/
public override execute( replacementText: string, result: ResultType ): void {
// 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).
//
// While this saving can be done in editing state, it's better to keep it here, as it's a part of the command logic
// and might be super tricky to implement in multi-root documents.
//
// 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
);

this._replace( replacementText, result );

// Let's revert the highlight offset to the previous value.
if ( this._state!.results.length ) {
// Highlight offset operates on sorted array, so we need to sort the results first.
// It's not guaranteed that items in state results are sorted, usually they are, but it's not guaranteed when
// the result is removed from the paragraph with other highlighted results.
const sortedResults = sortSearchResultsByMarkerPositions( this.editor.model, [ ...this._state!.results ] );

// Just make sure that we don't overflow the results array, so use modulo.
this._state!.highlightedResult = sortedResults[ oldHighlightOffset % sortedResults.length ];
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
Expand Down

0 comments on commit cb838d6

Please sign in to comment.