-
-
Notifications
You must be signed in to change notification settings - Fork 477
feat: Support default_values for Select of type ComponentType.mentionable_select and ComponentType.channel_select #2793
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
base: master
Are you sure you want to change the base?
Conversation
This commit expands handling of "default_values" to include objects like users, roles, channels, and threads in select menus other than string selects. It introduces type checks and conversions for these objects and ensures proper serialization to payloads.
Add support for creating SelectDefaultValue from dictionaries and improve serialization by converting default_values elements to dictionaries. Ensure consistency by using the enum value for type in payload generation.
raise InvalidArgument( | ||
"default_values can only be set on non string selects" | ||
) | ||
if not isinstance(value, list): |
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 about the necessity of these checks. value
is already typed. Wait for other feedback.
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.
Most of those checks are again similar to how the options work/are checked.
My argument for stricter type checking and not just relying on the typing would be the similarities to the options could lead to confusion between both and with those checks we can avoid some reports and provide a better exception
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 specific checks I'm talking about are the list
and all(isinstance)
ones. While I feel like the other might be justified, it feels redundant to have type checks with also typed parameters. Yes, to some extent it is good to have better exceptions, but python isn't strictly typed anyways, as of itself, and it feels a little arbitrary to add them here.
raise InvalidArgument( | ||
"default values have to be of type GuildChannel or Thread" | ||
) | ||
elif self.type is ComponentType.channel_select: |
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.
Maybe a dict mapping could be used for setting default_value_type
@NeloBlivion Would this interfere with #2707 ? |
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.
Minor changes and docs suggestions
Add this feature to the pre-existing select example.
Signed-off-by: Lukas Dobler <[email protected]>
I addressed most of the review comments, the two left open are something which need a final decision on what to do. @Paillat-dev @JustaSqu1d |
Summary
Looking at https://discord.com/developers/docs/components/reference#user-select I noticed that there is support for preselecting users, channels, roles, mentionables for the non string selects. This pr adds support for this.
Information
examples, ...).
Checklist
type: ignore
comments were used, a comment is also left explaining why.