Skip to content
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

feat: right-clickybois (context menu support for ApplicationCommand and CommandInteraction) #6176

Merged
merged 18 commits into from Aug 10, 2021

Conversation

monbrey
Copy link
Member

@monbrey monbrey commented Jul 22, 2021

This adds support for creating Application Commands in right-click context menus, and receiving the interactions.

The received data is parsed into the CommandOptionResolver as seamlessly as possible:

  • A command acting on a user creates a USER type option: getUser('user) / getMember('user')
  • A command acting on a message creates a fake _MESSAGE type option for the new getMessage('message')

The type of application command is yet to be included in the interaction data, but when it is this will be updated to use that rather than relying on the existence of target_id.

Status and versioning classification:

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating
  • This PR changes the library's interface (methods or parameters added)

src/structures/CommandInteraction.js Outdated Show resolved Hide resolved
src/structures/CommandInteraction.js Outdated Show resolved Hide resolved
@ThaTiemsz
Copy link
Contributor

ApplicationCommand is missing a type property.

@advaith1
Copy link
Contributor

imo it's a bad idea to stick the target information into options; currently context cmds can't have options but they might add support in the future, and also it's not really good ux to just make fake options. it would be better to have dedicated target properties; maybe interaction.target.id, interaction.target.message, interaction.target.user, and interaction.target.member?

@monbrey
Copy link
Member Author

monbrey commented Jul 26, 2021

Friendly reminder that draft PR means not ready for review

@monbrey monbrey force-pushed the context-menus branch 2 times, most recently from fc1db39 to 06f00a3 Compare August 3, 2021 23:47
@iCrawl iCrawl added this to the Version 13.1 milestone Aug 6, 2021
@iCrawl
Copy link
Member

iCrawl commented Aug 6, 2021

This needs a rebase.

@monbrey monbrey marked this pull request as ready for review August 7, 2021 07:11
Copy link
Member

@SpaceEEC SpaceEEC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small doc nit, otherwise LGTM

src/structures/ContextMenuInteraction.js Outdated Show resolved Hide resolved
@iCrawl iCrawl requested a review from vladfrangu August 7, 2021 15:41
Copy link
Contributor

@ThaTiemsz ThaTiemsz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ApplicationCommand#type should be added so that it's possible to check whether the command is a slash command or context menu.

@monbrey
Copy link
Member Author

monbrey commented Aug 7, 2021

That's only on Interaction though. You can't do that with commands that are cached that you've fetched from the API.

Right, I misread. Will do.

Copy link
Contributor

@ThaTiemsz ThaTiemsz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You forgot to update the ApplicationCommand types, otherwise LGTM!

src/structures/BaseCommandInteraction.js Outdated Show resolved Hide resolved
typings/index.d.ts Outdated Show resolved Hide resolved
@iCrawl
Copy link
Member

iCrawl commented Aug 10, 2021

This needs a rebase.

@iCrawl iCrawl merged commit 0266f28 into discordjs:main Aug 10, 2021
@monbrey monbrey deleted the context-menus branch August 15, 2021 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants