Skip to content

Commit

Permalink
Adjust tests, change editing instead of command
Browse files Browse the repository at this point in the history
  • Loading branch information
Mati365 committed Oct 22, 2024
1 parent 208cc4f commit 46d34be
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 29 deletions.
24 changes: 22 additions & 2 deletions packages/ckeditor5-find-and-replace/src/findandreplaceediting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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<Exclude<DiffItem, DiffItemAttribute>>;
const changedMarkers = model.document.differ.getChangedMarkers();

Expand Down Expand Up @@ -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();
Expand Down
26 changes: 0 additions & 26 deletions packages/ckeditor5-find-and-replace/src/replacecommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
*/

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

/**
Expand All @@ -23,31 +22,6 @@ 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 it's possible to fix it somewhere in the editing flow, it's easier and safer to just save the offset
// and restore the highlighted result after the changes. It'll not affect other find and replace features.
//
// 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 restore the next result to be highlighted. Keep in mind that offset stays the same.
// Keep in mind that highlight offset is references to sorted results, so we need to sort them first
// as it's done in refreshHighlightOffset method.
const restoreNextResult = sortSearchResultsByMarkerPositions( this._state.results ).get( oldHighlightOffset );

if ( restoreNextResult ) {
this._state.highlightedResult = restoreNextResult;
}
}
}
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 46d34be

Please sign in to comment.