-
-
Notifications
You must be signed in to change notification settings - Fork 57
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
discord: Added SelectedValue fields for Channel, Mentionable, User and Role SelectComponents #455
base: v3
Are you sure you want to change the base?
Conversation
discord/component.go
Outdated
@@ -822,6 +858,9 @@ type MentionableSelectComponent struct { | |||
ValueLimits [2]int `json:"-"` | |||
// Disabled disables the select if true. | |||
Disabled bool `json:"disabled,omitempty"` | |||
// SelectedMentions is the slice of discord.UserID's and discord.RoleID's | |||
// that are marked as selected by default | |||
SelectedMentions []interface{} `json:"-"` |
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.
If we're using a hidden field, then it's best to just have separate hidden DefaultUsers
and DefaultRoles
fields, then combine them in MarshalJSON
.
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 breaks proper order. There might be {(user, 2), (role, 3), (user, 1)}
sequence.
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 agree with @MCausc78. What's wrong with using plain interfaces? It's already documented above the field for the users.
I can add an example snippet in the comments, If it's not clear enough
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.
What's wrong with using plain interfaces? It's already documented above the field for the users.
It's not type-safe.
The only way I would agree to moving forward with this solution is to make custom SelectedUserID
and SelectedRoleID
types and a SelectableID
union type to combine the two. Arikawa tries its best to not have any ambiguous/error-prone any
types in its code, so this shouldn't be 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.
@diamondburned How about this implementation?
type DefaultMention struct {
userId *UserID `json:"-"`
roleId *RoleID `json:"-"`
}
// Creates new DefaultMention type with only UserID
func UserMention (userId UserID) DefaultMention {
return DefaultMention{userId: &userId}
}
// Creates new DefaultMention type with only RoleID
func RoleMention(roleId RoleID) DefaultMention {
return DefaultMention{roleId: &roleId}
}
and when implementing,
DefaultMentions: []discord.DefaultMention{ discord.UserMention(793688107077468171), discord.RoleMention(1299037008332849212) },
This PR adds the following fields to ChannelSelectComponent, MentionableSelectComponent and RoleSelectComponent,
SelectedRoles
forRoleSelectComponent
.SelectedChannels
forChannelSelectComponent
.SelectedMentions
forMentionableSelectComponent
.SelectedUsers
forUserSelectComponent
.All the fields will take in the slice of ID's of their kind and populates the SelectComponent beforehand.
Note
SelectedMentions
will take in the slice ofinterface{}
and later type-asserted to respective UserID / RoleID.If wanted to change this implementation, please lmk.