[MM-26655] Show auto-complete channel options when /join command suggestion is selected
#29227
Conversation
|
@mas-who: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsI understand the commands that are listed here |
| handleCompleteWord(term: string, pretext: string, callback: (s: string) => void) { | ||
| callback(term + ' '); | ||
| let newValue = term + ' '; | ||
| if (term === '/join') { |
There was a problem hiding this comment.
would it be worthwhile to convert this logic into something more generic, and also cater for other commands that have the same behaviour? e.g. /mute
There was a problem hiding this comment.
I think you could try to set a dictionary with command and the symbol to follow, then find the term in the dictionary and use the symbol to create the new value. Perhaps you can start with join and mute and see how it works. But feel free to open a new PR for that. This one is ok as it is, given that is solves what was initially requested.
hanzei
added
2: Dev Review
|
Not triggering E2E tests: this PR has 9 commit status(es) or check-runs that are not passing. Ensure all statuses aside from the E2E testing ones are green, before triggering E2E tests. |
mm-cloud-bot
added
release-note
| handleCompleteWord(term: string, pretext: string, callback: (s: string) => void) { | ||
| callback(term + ' '); | ||
| let newValue = term + ' '; | ||
| if (term === '/join') { |
There was a problem hiding this comment.
I think you could try to set a dictionary with command and the symbol to follow, then find the term in the dictionary and use the symbol to create the new value. Perhaps you can start with join and mute and see how it works. But feel free to open a new PR for that. This one is ok as it is, given that is solves what was initially requested.
@pvev noted, I think let's get this PR merged and I will open a new issue referencing this one to generalise the approach? |
devinbinnie
requested review from
hmhealey
and removed request for
devinbinnie
|
Passing this over to @hmhealey as he understands the inner workings of the |
hmhealey
changed the title
/join command suggestion is selected/join command suggestion is selected
hmhealey
left a comment
hmhealey
left a comment
There was a problem hiding this comment.
Thanks for the PR, @mas-who!
I like the idea of trying to add this when completing the word, but it seems like this won't work because handleCompleteWord is triggered after the text in the textbox is updated. I'm actually surprised that the changes to handleCompleteWord make the autocomplete appear even though there's no ~ in the textbox.
Instead, I think it'd be better to change this in CommandProvider.handleWebapp. When it calls Client4.getCommandAutocompleteSuggestionsList to make the array of results from the server, I think that's where we should look for the result that corresponds to the /join command and add a ~ to it. That way, it'll keep the special handling for /join inside the CommandProvider without needing special handling for it in SuggestionBox. Ideally, we'll keep all of that logic in the provider itself.
How does that sound?
| const suffix = text.substring(caret); | ||
| let suffix = text.substring(caret); | ||
| if (term.endsWith('join')) { | ||
| suffix = '~'; |
There was a problem hiding this comment.
We don't want to use the suffix for this because that contains all the pre-existing text after the keyboard caret. For example, if the user goes back to add an emoji earlier in this text
this : is some text
^ the caret is after this colon
then the suffix will be is some text
| window.requestAnimationFrame(() => { | ||
| if (textbox.value === newValue) { | ||
| Utils.setCaretPosition(textbox, prefix.length + term.length + 1); | ||
| Utils.setCaretPosition(textbox, prefix.length + term.length + suffix.length + 1); |
There was a problem hiding this comment.
Similarly, this would make it so that, in cases where we keep the suffix, the cursor would move to the end of the textbox instead of after the autocompleted text
|
This PR has been automatically labelled "stale" because it hasn't had recent activity. |
8 participants
Summary
This PR is aimed resolve the issue raised in #15755 . The approach is to add the
~character as an suffix when the/joincommand is selected via mouse or keyboard (Enter or Tab). By doing this, it would automatically display the auto-complete options for the/joincommand when selected.QA steps:
/so that the/joincommand is displayed in the auto-complete suggestions./joincommand in the suggestion list by clicking it, or pressing theTab/Enterkey./joincommand using anyone of the interactions mentioned in 2 should result in/join ~displayed in the message input, and the suggestion list for channel options should be displayed automatically.Ticket Link
Fixes #15755
https://mattermost.atlassian.net/browse/MM-26655
Screenshots
Release Note