-
Notifications
You must be signed in to change notification settings - Fork 82
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
Implement select menus #339
Conversation
set(value) { | ||
field = value | ||
if (value > maximumValues) { | ||
maximumValues = value |
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.
Why do we check here instead of throwing
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.
For one we generally don't throw in builders. I thought it'd be more intuitive that something like this would work:
selectMenu(id) {
minValues = 2
option("a", "key.a")
option("b", "key.b")
option("c", "key.c")
option("d", "key.d")
}
If you think that min and max values should always be set instead when raising the minimum above 1
then I'll remove the behaviour.
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.
honestly, it is kinda hard to know what would be the best approach, I mean, the request would still be valid
using an IntRange is another possible solution, and finally an 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.
Sounds to me like an IntRange would be safe to use. We take the min value for our min_value and likewise for max. it also ensure that one is equal or less than the others. allows combos like 1..3, 1..1, 0..0, 2.downTo 5 etc..
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.
Good idea, I'll apply the suggestion
* Add a cache aware rest entity supplier (#338) * add a cahe aware rest supplier/strategy * abstract the new flow creation for storeAndEmit * apply suggestions * Reduce the number of iterations rename storeAndEmit to storeOnEach * update changelog * Fix master gateway average ping conversion (#337) * Change field visibility and single-expression return syntax (#335) * changed the visibility for the name, descript.. and type field, changed the toRequest functions to single expression-styled returns * remade .name and .description to public upon request, kept .type protected * remade .type into read only (val) * Implement select menus (#339) * Add select menu json representation * Add select menu builders * add serialization tests for select menu interactions * Add core implementation of select menus * Update ButtonComponent docs * Make SelectMenu min/max values a range * Fix getGuildBanOrNull recursion (#341) Fixes the self call in getGuildBanOrNull by delegating the call to the supplier instead * Make UpdateMessageInteractionResponseCreateBuilder fields nullable (#340) Makes the fields in the builder and request nullable where appropriate for updating a message * improve readme * api dump * add breaking changes to changelog * mention the type visibility change [ci skip] Co-authored-by: Bart Arys <[email protected]> Co-authored-by: Noak Palander <[email protected]>
This PR adds support for select menus.
Component
andComponentInteraction
were broken to add support for them.