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

[Feature]: ComponentBuilder component removal methods #2644

Merged
merged 5 commits into from
Aug 10, 2023

Conversation

cjbonn
Copy link
Contributor

@cjbonn cjbonn commented Mar 30, 2023

This PR adds two helper methods to ComponentBuilder to assist in the removal of components.

  • ComponentBuilder.RemoveComponent(customId) will remove a component based on its custom id.
  • ComponentBuilder.RemoveComponentsOfType(componentType) will remove all components of a given type (ie: button, menu, etc).

Build() will now automatically resolve empty ActionRows.

An example that will always remove the button that is clicked from a message:

[ComponentInteraction("testBtn:*", ignoreGroupNames: true)]
public async Task HandleButtonClick(string id) {
      ComponentBuilder comp = ComponentBuilder.FromMessage((Context.Interaction as IComponentInteraction).Message);
      comp.RemoveComponent($"testBtn:{id}");
      await (Context.Interaction as IComponentInteraction).UpdateAsync(m => { m.Components = comp.Build(); });
}

@Misha-133
Copy link
Member

IMO it would be a good idea to add a method to remove a link-button by a URL, since one doesn't have a custom id

@cjbonn
Copy link
Contributor Author

cjbonn commented Mar 30, 2023

IMO it would be a good idea to add a method to remove a link-button by a URL, since one doesn't have a custom id

Good call. What would you suggest an example implementation looks like? RemoveButtonByURL(url)?

@Misha-133
Copy link
Member

Misha-133 commented Mar 30, 2023

RemoveButtonByURL(url)

Looks good to me
👍

@cjbonn
Copy link
Contributor Author

cjbonn commented Mar 30, 2023

@Misha-133 not sure I can/want to do the link button one - IMessageComponent only exposes custom id & component type to reference. Adding to that interface would mean forcing an additional unused property in all the other components that don't need it (ie: SelectMenu, TextInput, etc).

@Misha-133
Copy link
Member

@Misha-133 not sure I can/want to do the link button one - IMessageComponent only exposes custom id & component type to reference. Adding to that interface would mean forcing an additional unused property in all the other components that don't need it (ie: SelectMenu, TextInput, etc).

You could cast the component to a button tho
something like

if(component is ButtonComponent button && button.Url == ......)

@cjbonn
Copy link
Contributor Author

cjbonn commented Mar 30, 2023

I was on the right track, just derped and used as instead of is, forgetting that it can be any component in there. This is why I don't code in the morning :')

@Misha-133
Copy link
Member

Also, shouldn't these methods support regex wildcards? So it'd be possible to do something like RemoveButton("buttonId_*")

@cjbonn
Copy link
Contributor Author

cjbonn commented Mar 30, 2023

Also, shouldn't these methods support regex wildcards? So it'd be possible to do something like RemoveButton("buttonId_*")

I despise working with Regex, but I'll take a gander when I'm more free and want to torture myself with it. 🥲

@cjbonn
Copy link
Contributor Author

cjbonn commented Mar 31, 2023

I took a look, figured I could just reference the Regex implementation that is used in the ComponentInteration attribute, but I cannot find it 😳 thinking we should just use this PR as-is, and have another PR sometime in the future that will expand the functionality of these to support regex.

@cjbonn cjbonn requested a review from csmir April 19, 2023 20:02
@csmir
Copy link
Collaborator

csmir commented Apr 19, 2023

A feature that does not support functionality the library offers out of the box should not be merged. @Cenngo, could you reference the component regex?

@cjbonn
Copy link
Contributor Author

cjbonn commented Apr 19, 2023

see https://github.com/discord-net/Discord.Net/blob/dev/src/Discord.Net.Interactions/Utilities/RegexUtils.cs#L94-L157

This is internal to the Discord.Net.Interaction namespace; ComponentBuilder is in Core... should I just copy all of the non-dependent code into either Core/Utils or ComponentBuilder.cs? or is there a better way to go about this?

@Cenngo
Copy link
Collaborator

Cenngo commented Apr 19, 2023

you could add the .Remove methods to the Discord.Interactions assembly as extension methods

@cjbonn
Copy link
Contributor Author

cjbonn commented Apr 19, 2023

you could add the .Remove methods to the Discord.Interactions assembly as extension methods

True, but along the lines of reusing that code, it is dependent on ICommandInfo and InteractionServiceConfig.WildCardExpression, both of which I don't think an extension method (or otherwise) would have any context for?

@cjbonn
Copy link
Contributor Author

cjbonn commented Apr 29, 2023

Alright, well I hoped to add a couple useful functions that assisted me in my project so others could have that functionality too, rather than not at all - but this is not something I intend to spend hours digging into, I have enough on my plate as it is. If this absolutely cannot be included without regex support and be incrementally developed to add regex later down the line, then please go ahead and close this PR. thx

@quinchs quinchs merged commit d5d7378 into discord-net:dev Aug 10, 2023
@cjbonn cjbonn deleted the ComponentBuilder-Helpers branch August 10, 2023 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants