Feat/components v2

[DRSchlaubi]

No description provided.

[lukellmann]

review of common

[zTrap]

Suggested change

	public fun separator(spacing: SeparatorSpacingSize): Unit = separator {
	public fun separator(
	spacing: SeparatorSpacingSize? = null,
	divider: Boolean? = null,
	): Unit = separator {
	this.spacing = spacing
	this.divider = divider
	}

[DRSchlaubi]

I don't really see the point of that, you can just call the seperator function with the builder yourself

[zTrap]

I agree, but there is no point to have partial member function. My opinion is "all or nothing" :D

[ptitjes]

Hey @DRSchlaubi ! Nice PR. I had to make a few tiny changes to make it work here:

• in the common module, regenerate code and dump API again
• in the rest module, dump API again
• make DiscordComponent.description both Optional and nullable [1]

You can see the details in the last three commits here:
https://github.com/ptitjes/kord/commits/feat/components-v2-didier/

[1] When deserializing the incoming messages in the Gateway, the description field sent back by Discord was null, and this was creating a deserialization error.

[lukellmann]

I haven't had time to look into things in detail yet, but the presence of AbstractMessageCreateBuilder and AbstractMessageModifyBuilder is a strong indictment of Kord's development philosophy, imo.

These types aren't new and weren't even touched in this PR, they have been there since #891

This approach makes no sense from a logical viewpoint, and will significantly complicate things for API consumers - now instead of having a single base type that already worked everywhere, I need to mix between the interface and abstract type depending on whether I want to use components or not, and I need to go back over every single piece of code that currently uses the interface to make sure it doesn't now actually need the abstract type.

I couldn't find anything you can do with the Abstract* classes that you can't do with the interfaces. Did you need to use the Abstract* classes in some code? You shouldn't have to.

[lukellmann]

This deprecation isn't actionable since this extension is more specific than ComponentContainerBuilder.actionRow (MessageBuilder implements that interface). Just don't deprecate it or use @LowPriorityInOverloadResolution. Should also add replaceWith and explicit level when going with the deprecation.

[Galarzaa90]

Is there something that can be helped with regarding this PR?

Would love to see this implemented.

[DRSchlaubi]

#1019, #1018 need fixing

[NotJansel]

tested around a bit and so far there was no error during the entire testing that could have been related to components v2

Edit: found out the hard way that a section cant have more than 3 text displays.

[NoComment1105]

Realised I never actually read this through, Looks good to me, one comment from Luke about deprecation that I agree with but other than that perfect :D

[honkling]

Is this PR only targeting Components V2 in messages?
I tried out the branch for my discord bot because I'd like to use a string select component in my modal, but it seems like I can only attach action rows to my modal (which limits me to text inputs only), and labels aren't supported.

[NotJansel]

@honkling to my knowlede yes, as the modal stuff got added pretty much right after this pr was ready for review/merge

[honkling]

Hmm. Then, should components v2 support for modals be expected in a future PR? Or will this PR expand scope to accommodate modals?