feat: Improving the UX of Slash Commands #36949
Conversation
🦋 Changeset detectedLatest commit: 41c689d The changes in this PR will be included in the next version bump. This PR includes changesets to release 39 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
|
WalkthroughUpdated slash-command parsing to require a colon-delimited label before parameters, adjusted composer popup selection/replacement to insert formatted params for slash commands, retained provider logic with whitespace-only edits, and added a standalone textual documentation file unrelated to runtime code. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant C as Composer Box
participant P as Commands Popup
participant S as Slash Parser
U->>C: Type "/cmd ..."
C->>P: Request suggestions (trigger="/")
P-->>C: Suggest command items
U->>P: Select command
P-->>C: Selected item (with params)
note over C: If trigger is "/"<br/>append "label: " formatting<br/>based on item.params
C->>C: Insert "/cmd <label>: "
U->>C: Fill parameters
C->>S: Submit message text
S->>S: Parse with ^\/([^\s]*)\s*[^:]*:\s*(.*)$
S-->>C: Extract cmd and params
C-->>U: Proceed with command handling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
apps/meteor/client/views/room/composer/hooks/useComposerBoxPopup.ts (3)
18-24: Avoid widening to a separate IItem; keep generics consistent or use intersection.Introducing
IItemforces casts (as T) and weakens type safety. PreferT & { params?: string; description?: string; permission?: string }for this hook’s internal usage.-interface IItem { - _id: string; - sort?: number; - description?: string; - params?: string; - permission?: string; -} +type WithCommandMeta = { params?: string; description?: string; permission?: string };And then type the handler as
(item: T & WithCommandMeta) => voidto drop the casts.
95-102: Type cast hides real mismatches.
commandsRef.current.select(item as T)compiles but can mask runtime shape issues. If you adopt the intersection type above, you can passitemdirectly without casting.
124-126: Cursor placement UX (optional).Consider placing the caret after the inserted “@”/“#” when present so users can immediately type the identifier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/meteor/client/lib/chats/flows/processSlashCommand.ts(1 hunks)apps/meteor/client/views/room/composer/hooks/useComposerBoxPopup.ts(4 hunks)apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx(1 hunks)h Commandq(1 hunks)
🔇 Additional comments (3)
apps/meteor/client/lib/chats/flows/processSlashCommand.ts (1)
55-64: Behavioral check: unparsed “/…” messages are sent as text.Given the current early-return on no match, any “/cmd foo” without a colon would bypass slash handling and be sent as plain text. After the parser fix above, this becomes moot, but please confirm there’s no path that still returns
undefinedfor valid commands.apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx (2)
332-334: No functional changes in this hunk. LGTM.Whitespace-only tweak around the permission check.
371-386: Slash preview regex still assumes “/cmd ” (space), not “label:”.
matchSelectorRegex: /(?:^)(\/[\w\d\S]+ )[^]*$/anchors on a trailing space after the command. If the composer now inserts “: …”, ensure previews still trigger as expected.
| // This regex matches strings starting with '/', captures the command name (non-whitespace) skips optional spaces and a label before a colon, and finally captures the parameters after the colon. | ||
| const match = msg.match(/^\/([^\s]*)\s*[^:]*:\s*(.*)$/); | ||
| if (!match) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const [, cmd, params] = match; | ||
| const cmd = match[1]; | ||
| const params = match[2]; | ||
|
|
There was a problem hiding this comment.
Colon-required parsing breaks existing slash commands and mis-parses URLs; make label optional and strip only when it matches the command’s declared param label.
Requiring a “label:” after the command means inputs like “/giphy cat” or “/invite @user” no longer parse and will be sent as plain text. It can also eat “http:” in “/topic http://…”. Additionally, ([^\s]*) permits an empty command.
Fix: parse like before, then conditionally drop a leading “:” only if it matches the registered command’s params (handling “@”/“#” forms), keeping full backward compatibility.
Apply this diff within parse:
- // This regex matches strings starting with '/', captures the command name (non-whitespace) skips optional spaces and a label before a colon, and finally captures the parameters after the colon.
- const match = msg.match(/^\/([^\s]*)\s*[^:]*:\s*(.*)$/);
+ // Match "/<cmd> <params...>" and keep params intact; we'll strip an optional "<label>:" only
+ // if it matches the command's declared params (e.g., "channel: #", "username: @").
+ const match = msg.match(/^\/(\S+)\s*(.*)$/);
if (!match) {
return undefined;
}
-
- const cmd = match[1];
- const params = match[2];
+ const cmd = match[1];
+ let params = match[2] ?? '';
+
+ // If this is a known command with a declared params label, strip a leading "<label>:"
+ // that the composer may have inserted (e.g., "channel: #", "username: @").
+ const meta = slashCommands.commands[cmd];
+ if (meta && typeof meta !== 'string' && typeof meta.params === 'string' && meta.params) {
+ const raw = meta.params;
+ const label = raw.startsWith('@') || raw.startsWith('#') ? raw.slice(1) : raw;
+ if (label) {
+ // escape regex meta chars in label
+ const esc = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+ const re = new RegExp(`^\\s*(?:${esc(label)})\\s*:\\s*`);
+ if (re.test(params)) {
+ params = params.replace(re, '');
+ }
+ }
+ }Optional (outside this hunk): import escapeRegExp instead of the inline esc helper if preferred.
// at top-level imports
import { escapeHTML /*, escapeRegExp*/ } from '@rocket.chat/string-helpers';🤖 Prompt for AI Agents
In apps/meteor/client/lib/chats/flows/processSlashCommand.ts around lines 13-21,
the current regex forces a required "label:" and allows an empty command which
breaks commands and mis-parses URLs; change parsing to first match a slash
command with a non-empty name and optional trailing text (no required colon),
then if a registered command exists check its declared params and only strip a
leading "<label>:" when that label exactly matches the command's param label
(handle optional "@" or "#" prefixes and use a safe escaped comparison); ensure
fallback returns the whole trailing text unchanged for backward compatibility
and prevent empty command names by using a non-empty capture for the command;
optionally import escapeRegExp from @rocket.chat/string-helpers to safely build
the label match.
| // This regex matches strings starting with '/', captures the command name (non-whitespace) skips optional spaces and a label before a colon, and finally captures the parameters after the colon. | ||
| const match = msg.match(/^\/([^\s]*)\s*[^:]*:\s*(.*)$/); |
There was a problem hiding this comment.
Require at least one non-space char for the command token.
Change ([^\s]*) to (\S+) to avoid matching “/ label: …” with an empty command.
-const match = msg.match(/^\/([^\s]*)\s*[^:]*:\s*(.*)$/);
+const match = msg.match(/^\/(\S+)\s*(.*)$/);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // This regex matches strings starting with '/', captures the command name (non-whitespace) skips optional spaces and a label before a colon, and finally captures the parameters after the colon. | |
| const match = msg.match(/^\/([^\s]*)\s*[^:]*:\s*(.*)$/); | |
| // This regex matches strings starting with '/', captures the command name (non-whitespace), skips optional spaces and a label before a colon, and finally captures the parameters after the colon. | |
| const match = msg.match(/^\/(\S+)\s*[^:]*:\s*(.*)$/); |
🤖 Prompt for AI Agents
In apps/meteor/client/lib/chats/flows/processSlashCommand.ts around lines 13-14,
the regex allows an empty command token because it uses ([^\s]*); update the
pattern to require at least one non-space character by replacing ([^\s]*) with
(\S+), i.e. change the regex to use (\S+) for the command capture so inputs like
"/ label: ..." no longer match as an empty command.
| // formattedParams formats command parameters by detecting '@' or '#' prefixes | ||
| const formattedParams = | ||
| item.params?.startsWith('@') || item.params?.startsWith('#') | ||
| ? `${item.params.slice(1)}: ${item.params.charAt(0)}` | ||
| : `${item.params}: `; | ||
| chat?.composer?.replaceText( | ||
| (option.prefix ?? option.trigger ?? '') + | ||
| option.getValue(item as T) + | ||
| (option.suffix ?? '') + | ||
| (option.trigger === '/' ? formattedParams : ''), | ||
| { | ||
| start: value.lastIndexOf(result[1] + result[2]), | ||
| end: chat?.composer?.selection.start, | ||
| }, | ||
| ); |
There was a problem hiding this comment.
Guard against “undefined: ” being inserted for commands without params; also trim and handle “@/#” cleanly.
When item.params is absent, the else-branch inserts undefined: into the composer. This is user-facing.
Apply this diff:
- // formattedParams formats command parameters by detecting '@' or '#' prefixes
- const formattedParams =
- item.params?.startsWith('@') || item.params?.startsWith('#')
- ? `${item.params.slice(1)}: ${item.params.charAt(0)}`
- : `${item.params}: `;
+ // Format an optional "<label>: " or "<label>: @|#"
+ const rawParams = item.params?.trim();
+ const formattedParams =
+ rawParams
+ ? (rawParams.startsWith('@') || rawParams.startsWith('#')
+ ? `${rawParams.slice(1)}: ${rawParams.charAt(0)}`
+ : `${rawParams}: `)
+ : '';
chat?.composer?.replaceText(
(option.prefix ?? option.trigger ?? '') +
option.getValue(item as T) +
(option.suffix ?? '') +
- (option.trigger === '/' ? formattedParams : ''),
+ (option.trigger === '/' ? formattedParams : ''),Optional: only append formattedParams when truthy to avoid stray spaces.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // formattedParams formats command parameters by detecting '@' or '#' prefixes | |
| const formattedParams = | |
| item.params?.startsWith('@') || item.params?.startsWith('#') | |
| ? `${item.params.slice(1)}: ${item.params.charAt(0)}` | |
| : `${item.params}: `; | |
| chat?.composer?.replaceText( | |
| (option.prefix ?? option.trigger ?? '') + | |
| option.getValue(item as T) + | |
| (option.suffix ?? '') + | |
| (option.trigger === '/' ? formattedParams : ''), | |
| { | |
| start: value.lastIndexOf(result[1] + result[2]), | |
| end: chat?.composer?.selection.start, | |
| }, | |
| ); | |
| // Format an optional "<label>: " or "<label>: @|#" | |
| const rawParams = item.params?.trim(); | |
| const formattedParams = | |
| rawParams | |
| ? (rawParams.startsWith('@') || rawParams.startsWith('#') | |
| ? `${rawParams.slice(1)}: ${rawParams.charAt(0)}` | |
| : `${rawParams}: `) | |
| : ''; | |
| chat?.composer?.replaceText( | |
| (option.prefix ?? option.trigger ?? '') + | |
| option.getValue(item as T) + | |
| (option.suffix ?? '') + | |
| (option.trigger === '/' ? formattedParams : ''), | |
| { | |
| start: value.lastIndexOf(result[1] + result[2]), | |
| end: chat?.composer?.selection.start, | |
| }, | |
| ); |
🤖 Prompt for AI Agents
In apps/meteor/client/views/room/composer/hooks/useComposerBoxPopup.ts around
lines 113–127, the current logic can insert "undefined: " when item.params is
missing and doesn't trim or handle '@'/'#' cleanly; update formattedParams to
first guard that item.params is a truthy string, trim it, then if it starts with
'@' or '#' strip the prefix and produce "stripped: prefix" (or just the stripped
string followed by ': ' if that exact format is required), otherwise produce
"trimmed: " only when trimmed is non-empty; finally, only append formattedParams
to the replacement string when it is a non-empty string to avoid stray spaces or
"undefined" appearing in the composer.
h Commandq
Outdated
|
|
||
| SSUUMMMMAARRYY OOFF LLEESSSS CCOOMMMMAANNDDSS | ||
|
|
||
| Commands marked with * may be preceded by a number, _N. | ||
| Notes in parentheses indicate the behavior if _N is given. | ||
| A key preceded by a caret indicates the Ctrl key; thus ^K is ctrl-K. | ||
|
|
||
| h H Display this help. | ||
| q :q Q :Q ZZ Exit. | ||
| --------------------------------------------------------------------------- | ||
|
|
||
| MMOOVVIINNGG | ||
|
|
||
| e ^E j ^N CR * Forward one line (or _N lines). | ||
| y ^Y k ^K ^P * Backward one line (or _N lines). | ||
| f ^F ^V SPACE * Forward one window (or _N lines). | ||
| b ^B ESC-v * Backward one window (or _N lines). | ||
| z * Forward one window (and set window to _N). | ||
| w * Backward one window (and set window to _N). | ||
| ESC-SPACE * Forward one window, but don't stop at end-of-file. |
There was a problem hiding this comment.
Remove stray “less” help document from the repository.
This appears unrelated to the PR and may carry licensing/maintenance baggage. Please drop it from the commit or relocate to appropriate docs if truly needed.
🤖 Prompt for AI Agents
In file "h Commandq" lines 1-20, this stray "less" help document should be
removed from the PR: delete the file from the repository (git rm --cached or git
rm as appropriate) or move it into the proper docs directory if it is
intentionally required, update any references and the commit (amend or create a
new commit) so the removal appears in the branch, and run a quick git
status/ls-tree to confirm the file is no longer included in the PR.
Salvatoree07
force-pushed
the
develop
branch
from
48b405d to
ce7cfc8
Compare
Proposed changes (including videos or screenshots)
I updated two main files: processSlashCommand.ts and useComposerPopupBox.tsx. The changes ensure that parameters required by slash commands are now displayed in the composer box, making it easier for users to see what inputs are needed.
Additionally, I modified the regex logic to better extract parameters from the input string in the composer box.
User Experience Improvements
Thanks to these changes, new users can use slash commands more easily, even if they are not familiar with Rocket.Chat’s features. These updates also lay the groundwork for adding new elements to the composer box to further improve the slash command user experience. Note: this resolves only part of a broader issue, which will require additional work for complete implementation.
Video.per.issue.su.Slash.Commands.mp4
Issue(s)
965 form feature-request repository
RocketChat/feature-requests#965
Steps to test or reproduce
Further comments
I chose this approach because I believe it’s the most intuitive way to redesign the slash command UX. However, further improvements could be made, such as highlighting parameter names and making them non-editable.
In addition, the system for handling slash commands with multiple parameters can be enhanced in future iterations.
Summary by CodeRabbit
New Features
Documentation