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: add support for autocomplete interactions #6672

Merged
merged 59 commits into from Oct 28, 2021
Merged

feat: add support for autocomplete interactions #6672

merged 59 commits into from Oct 28, 2021

Conversation

OfficialSirH
Copy link
Contributor

@OfficialSirH OfficialSirH commented Sep 22, 2021

Please describe the changes this PR makes and why it should be merged:

Status and versioning classification:

  • Code changes have been tested against the Discord API
  • I know how to update typings and have done so
  • This PR changes the library's interface (methods or parameters added)

src/structures/AutocompleteInteraction.js Show resolved Hide resolved
src/structures/CommandInteractionOptionResolver.js Outdated Show resolved Hide resolved
src/structures/Interaction.js Outdated Show resolved Hide resolved
src/structures/interfaces/InteractionResponses.js Outdated Show resolved Hide resolved
src/structures/interfaces/InteractionResponses.js Outdated Show resolved Hide resolved
src/structures/AutocompleteInteraction.js Show resolved Hide resolved
typings/index.d.ts Outdated Show resolved Hide resolved
typings/index.d.ts Outdated Show resolved Hide resolved
src/structures/Interaction.js Outdated Show resolved Hide resolved
src/structures/CommandInteractionOptionResolver.js Outdated Show resolved Hide resolved
@greysdawn
Copy link

Do you think it would be better for getFocused() to return the whole option instead of just the value of it? Seeing as multiple options can have autocomplete on (afaik) and devs will probably want to know which out of all the options is currently focused using that

I know you can also just check if an individual option is .focused but I feel like making getFocused() shortcut to the full option, or at least including its name, might be better 🤔

Copy link
Member

@ckohen ckohen left a comment

Choose a reason for hiding this comment

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

This PR should also be adding support for the autocomplete parameter in ApplicationCommand.options[n]!

src/structures/AutocompleteInteraction.js Outdated Show resolved Hide resolved
src/structures/AutocompleteInteraction.js Outdated Show resolved Hide resolved
src/structures/CommandInteractionOptionResolver.js Outdated Show resolved Hide resolved
@ImRodry
Copy link
Contributor

ImRodry commented Sep 22, 2021

Do you think it would be better for getFocused() to return the whole option instead of just the value of it? Seeing as multiple options can have autocomplete on (afaik) and devs will probably want to know which out of all the options is currently focused using that

I know you can also just check if an individual option is .focused but I feel like making getFocused() shortcut to the full option, or at least including its name, might be better 🤔

As for this it could probably be toggleable through a parameter like sendFull and, like one of the discussions above mentioned, the required parameter should be removed since it doesn't make much sense to be there, and should just be replaced by this one IMO

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

iCrawl commented Oct 12, 2021

This needs a rebase.

@iCrawl
Copy link
Member

iCrawl commented Oct 26, 2021

This needs a rebase and got a new upstream PR:

Can you double check if anything changed and make changes accordingly?

@OfficialSirH
Copy link
Contributor Author

It all looks good, I don't see any changes with it.

@ImRodry
Copy link
Contributor

ImRodry commented Oct 27, 2021

This needs a new rebase btw

@suneettipirneni
Copy link
Member

The upstream PR was updated again today, and it was indicated that the recieved option values could be partial. Djs should probably also represent that.

@ImRodry
Copy link
Contributor

ImRodry commented Oct 27, 2021

Interaction partial type perhaps?

@samarmeena
Copy link
Contributor

@OfficialSirH discord/discord-api-docs#3996 merged, please rebase and fix conflicts.

typings/index.d.ts Outdated Show resolved Hide resolved
typings/index.d.ts Outdated Show resolved Hide resolved
@suneettipirneni
Copy link
Member

@OfficialSirH I've made a PR for your branch to fix all of the type test errors:

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