-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
Context menu, checks, and cooldowns guide #7952
base: docs/guide
Are you sure you want to change the base?
Conversation
Added the checks guide, which includes the information on cooldowns. This guide is specifically for AppCommand checks, as there are differences from standard checks. Not sure if it deserves its own page, but the document is written up now and can be put wherever later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job!
Co-authored-by: GoogleGenius <[email protected]>
Co-authored-by: GoogleGenius <[email protected]>
Co-authored-by: GoogleGenius <[email protected]>
Co-authored-by: GoogleGenius <[email protected]>
Thanks for catching stuff I missed! |
Just did a quick scan of this PR. Global interaction checks are not documented yet, which I will probably do in a bit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The guide itself looks pretty good, however the rst files need to be moved into a guide
folder, eg /docs/guide/interactions/
.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will probably the last of suggestions i'll do for this pr. once all of the suggestions and edits have been made, you can probably request the final review for danny and that should be it
Co-authored-by: Willy <[email protected]> Co-authored-by: Noelle Wang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not entirely sure about owner_only things being for guild owners only but aside from that, some weird line wrapping and my wording nits looks good
|
||
- Context menus can be triggered from two possible sources: by right clicking on a user or server member, or by right clicking on a message. | ||
- The ``name`` argument of the :func:`@context_menu <.app_commands.context_menu>` decorator defines the command name that appears in the context menu. | ||
- The second argument can be of type :class:`.User`, :class:`.Member`, or :class:`.User` | :class:`.Member` for user context menus. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not obvious to me reading this what the distinction between User, Member or User | Member is, should this not just be User or Member and drop the union at the end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how to specify this one honestly.
To my knowledge, when used in a Guild context, the arg will be a Member, and when used in a DM context, it will be a User. However, you can also get a User object for both Guild and DMs, or use the union to get either one respective to what environment you invoke the command/menu in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, makes a bit more sense now, not sure if there's a better way to put this, I'll leave that to someone else
Added most of the review suggestions from @Gobot1234 Co-authored-by: James Hilton-Balfe <[email protected]>
@app_commands.context_menu(name='User Context') | ||
@app_commands.default_permissions(manage_messages=True, manage_guild=True) | ||
async def default_permissions_slash_command(interaction: discord.Interaction, user: discord.User) -> None: | ||
await interaction.response.send_message('You may or may not have those permissions!' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
await interaction.response.send_message('You may or may not have those permissions!' | |
await interaction.response.send_message('You may or may not have those permissions!') |
Summary
Updating the Context Menus guide (Just something I threw together as it is already being worked on by another as well).
I plan to also specifically add an
app_commands.checks
guide, which will include information on cooldowns.Checklist