Handle spaces in feeds for mention plugin #11017
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CatStrategist
changed the title
niegowski
approved these changes
Dec 21, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Suggested merge commit message (convention)
Fix (mention): Mention plugin now allows searching mentions that include space character. Closes #9741.
Additional information
Mention plugin allowed user to search for feeds only using single words and as soon as it encountered a space it closed items list with results even if there were matches including a space e.g. 'Barry Thompson'. It happened because of regexp for matching mentions in currently edited text that only allowed visible characters to occur after a mention mark.
At first it seemed as an easy improvement of regexp to just allow a space but a scenario with multiple registered markes complicated it a lot.
Take this case:
Regexp allowing space would match @foo as well as #bar. After playing a little bit with regexp to handle that case it quickly got overcomplicated and still had some other edge cases so I had to withdraw from this approach.
Previously regexp to match mention tested entire text node till caret but what we really needed to test is just a text since last valid marker in text. But yet again, case with multiple markers complicated it a little bit. Previously there was a
TextWatcherfor each marker that tried to match a mention for it's own marker. It run outside of state of mention plugin and executedmatchedorumatchedevents without knowledge of matches for other markers. It could find a match for one marker and tried to show UI, but the other marker firedunmatchedevent and immediately hid UI. Before going with solution for spaces it had to be fixed. When initializing plugin I've merged watchers to a single one who knows about each marker and runs only one on each text change. Having that in place, I could pass a test function toTextWatcherand find last valid marker in text, trim text since last marker (+1 a character behind to test if marker is in correct place) till caret. Having only that part the rest of the code could stay as usual with just a little change to regexp that now allows any character after marker character instead of just visible characters.