Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No longer jump to the first item after replacing phrase #17292

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Mati365
Copy link
Member

@Mati365 Mati365 commented Oct 21, 2024

Suggested merge commit message (convention)

Fix (find-and-replace): Find and replace no longer randomly jumps to the first found item after the replace operation. Closes #16648


Additional information

After removal of marker, we used to reset search to the first changed item. It's buggy because changed items refer to search results affected by the update algorithm, which used to change found elements, even if they were not replaced. It caused "random" jumps between items after replace.

The example might be this list of results:

obraz

I replaced one word in a paragraph containing few search results with 2, 3 and 4 indices. It updated item with 3 index, but it turns out that algorithm updated also items with 2 i 4 IDs. It's counterintuitive, and leaded to the incorrect assumption in the editing flow that we can reset to the first changed node when at least one yellow marker above the search result is removed. In this case, highlight item was pointed to 2 item instead of 4 item.

I enforced restoring old highlight offset after operations, so it should be fine now.

Before

find-and-replace-before-2024-10-22_07.28.59.mp4

After

find-and-replace-after-2024-10-22_07.26.36.mp4

@Mati365
Copy link
Member Author

Mati365 commented Oct 22, 2024

@charlttsie Can you take a look and perform a QA check on this change?

@juliaflejterska
Copy link

juliaflejterska commented Oct 28, 2024

Find and replace jumps to incorrect item in multi-root editor

Steps

  1. Open the multi-root editor with multiple roots containing paragraphs with several replaceable items.
  2. Use Find and replace feature to replace the 2nd item in the 1st root.

Result

After replacing the 2nd item in the 1st root, Find and replace jumps to the 2nd root and replaces the 2nd item:

multiroot.mov

@Mati365 Mati365 force-pushed the ck/16648 branch 2 times, most recently from 94b9ae4 to cb838d6 Compare October 29, 2024 07:20
@Mati365
Copy link
Member Author

Mati365 commented Oct 29, 2024

I pushed some changes, it turns out that results were not sorted correctly regarding multi root's order and were misaligned in state. Nice finding @juliaflejterska!

@charlttsie
Copy link
Contributor

We've finished testing the PR with @juliaflejterska, and it looks good to us. The initial issue is fixed, and we haven't found any regressions other than the issue with the multi-root editor, which no longer occurs after the recent changes 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Find and replace highlight sometimes jumps to the first item instead of next after replacing
3 participants