Autocompleters: Consider email address context in user mention #47249
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
|
Size Change: +42 B (0%) Total Size: 1.84 MB
ℹ️ View Unchanged
|
t-hamano
added
[Package] Editor
kevin940726
reviewed
Jan 30, 2023
| @@ -42,6 +42,11 @@ export default { | |||
| className: 'editor-autocompleters__user', | |||
| triggerPrefix: '@', | |||
|
|
|||
| allowContext( before ) { | |||
| // Triggers when at the beginning of a context or when the immediately preceding character is a space. | |||
| return /^$/.test( before ) || /^\s$/.test( before.slice( -1 ) ); | |||
There was a problem hiding this comment.
I wonder if something like a word boundary better represents the use case here? For instance:
Suggested change
| return /^$/.test( before ) || /^\s$/.test( before.slice( -1 ) ); | |
| return /\B$/.test( before ); |
This will allow user mentions to be triggered when characters like : precede them. I'm not familiar with the autocompleters code to know if it's true though.
There was a problem hiding this comment.
Thanks for bringing this PR to my attention.
I have tried to see how software and tools that have user mentions with the @ symbol behave when typing :@. Slack, Gmail, and GitHub did not trigger mentions with ":@", so I think it may be more natural to not depend on non-word boundaries.
There was a problem hiding this comment.
Thanks for testing it out! What about other languages such as Japanese or Chinese? Something like 我和@user will probably want to match user mention too. But they should also be valid emails, I guess there's no right answer here 😅 .
There was a problem hiding this comment.
What about other languages such as Japanese or Chinese?
I used the IME, a common input system for Japanese, to test what happens when the @ symbol is typed followed by Japanese characters. Interestingly, it had different results.
Slack launches the mention:
slack.mp4
GitHub doesn't launch the mention:
github.mp4
Gmail doesn't launch the mention (I will not post the video because it contains my personal email address 🙂)
Personally, given that Gmail allows non-Latin characters as usernames for email addresses, I would assume that the @ symbol following a non-Latin character is an email address and should not trigger a mentions.
There was a problem hiding this comment.
I guess we'll need second opinions on this. I don't have a strong preference here, but I'm also not the one most familiar with autocompletion either 😅 . We would probably also want to add some documentation if we decide to change its behavior too.
@ellatrix Do you have any insights about this? Or do you have someone in mind who does?
There was a problem hiding this comment.
Hi all,
in my opinion, the main problem is that an average user doesn't expect entering @something or [email protected] to autocomplete it from the user table of the website. I have seen people reference external social media accounts/bloggers with @, but never internal.
Why can't this be a feature of the search box that pops up when adding link to text?
Best regards,
Andrej
There was a problem hiding this comment.
Btw, what is the regex used in PHP that matches these user names? I guess we should match whatever that is doing.
There was a problem hiding this comment.
Are you referring to the feature where the WordPress username is automatically linked to the profile on the front end?
For example, if it is a forum, it seems that it is converted to a link via a proprietary plugin. It seems that they are expecting a space before the @.
I think there is also some sort of process for mentions on the Make WordPress Core blog, but I couldn't find where it was done.
There was a problem hiding this comment.
Oh, I didn't know we didn't do this in core.
There was a problem hiding this comment.
I don't have a strong opinion here tbh.
|
Flaky tests detected in 4e156fa. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5334210481
|
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
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.
Closes: #46348
What?
This PR prevents the autocompleters displaying user mentions from triggering in a context where it is considered an email address entry.
Why?
Entering
@after any text, as shown below, will activate the completer, even though it is considered an email address entry:fa3c86258064bf12d35b83829c653f1d.mp4
I think the completer should not be triggered in this context.
How?
I considered that user mentions would be made in the following cases:
@character is typed at the beginning of the text@character.Therefore, if this was not the case, I did not trigger the completer. I would expect that this logic is also consistent with the mentions in the GitHub comments.
73ad77534c407d060d8dc458703518e6.mp4
Testing Instructions
@character at the beginning of the paragraph block.email@is entered, it is considered to be an email address and the completer will not be triggered.You are @is entered, it is considered a user mentation and confirms that the completer will be triggered.bf051ec55197eb40b986957e0b872770.mp4