-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Fix autocomplete trigger character detection #55301
Conversation
Flaky tests detected in df24fddcdd4300df3669a29204c3b9e397250054. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6498655061
|
df24fdd
to
301f294
Compare
301f294
to
58fb334
Compare
@WunderBart, how are you accessing the |
Ah, good question! Here's what I do to use the env from that autocomplete test:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for those steps @WunderBart, I wanted to make sure we were testing the same things! I was actually noticing another odd behavior, but it doesn't look related to this PR at all:
When I first load the editor and start enter a user mention, the autocomplete options appear to re-render on each character, but only for the first time I try it. All subsequent uses of Autocomplete render once and just filter out options as I go. This looks like it happens on trunk and with either of the testing methods I used though, so again, I think it's completely unrelated.
Testing this PR using both your steps and my own approach to activating the secondary +
completer, this feels solid to me. The second completer consistently renders the options without dropping them on every other character like trunk currently does, and if I log out the currently selected completer as I go everything looks good 👍
What?
Fix trigger character detection when using multiple different triggers inline.
Screen.Recording.2023-10-12.at.17.54.50.mov
Screen.Recording.2023-10-12.at.17.51.44.mov
Closes #55118
How?
The function that finds a completer for given text content performs multiple checks before returning a matching completer. The main issue here is that there are multiple trigger characters, and if they're used side by side, the first one that matches all the criteria is returned. For the given example above, the completers are toggled between the one for
@
and the one for+
triggers as we type, that's why the popup sometimes disappears, as the matching string is different on each keystroke.We should not be relying on the order of the triggers array, so to address this, I've added a step where the completer is picked by the closest trigger character to the end of the current text content. After that, all the checks that were there before are performed as well to ensure there are no regressions.
Testing Instructions
Follow the steps to reproduce what has been recorded on the before and after videos above.
trunk
and follow the steps described in Mentions autocomplete is unstable #55118 to see the regression.@
,+
,All the E2E tests should pass.