Skip to content

Components v2 #7487

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

Merged
merged 1 commit into from
Apr 22, 2025
Merged

Components v2 #7487

merged 1 commit into from
Apr 22, 2025

Conversation

anthonydiscord
Copy link
Contributor

@anthonydiscord anthonydiscord commented Apr 22, 2025

Adding documentation, changelog, and links updates for components v2. The interactions.js library and samples using it are going to follow in another PR.

@anthonydiscord anthonydiscord requested a review from a team as a code owner April 22, 2025 01:48
@anthonydiscord anthonydiscord requested review from markmandel and colinloretz and removed request for a team and markmandel April 22, 2025 01:48
Copy link
Contributor

@Lulalaby Lulalaby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following message is repeated really often:

> info
> To use this component, you need to send the [message flag](/docs/resources/message#message-object-message-flags) `1 << 15` (IS_COMPONENTS_V2) which can be activated on a per-message basis.

Might it be better to just specify a version number in the heading, linking to "this needs flag xy"?

Anyways, I really love how the docs turned out. Might've overseen something. Will look a bit later again.
The suggestions should catch most > info parts and change them to :::info as Justin mentioned.
Also discussion point: Putting IS_COMPONENTS_V2 into ` IS_COMPONENTS_V2

Since I was asked to review

Comment on lines +52 to +56
| 13 | [File](/docs/components/reference#file) | Displays an attached file | Content | Message |
| 14 | [Separator](/docs/components/reference#separator) | Component to add vertical padding between other components | Layout | Message |
| 17 | [Container](/docs/components/reference#container) | Container that visually groups a set of components | Layout | Message |
Copy link
Contributor

@Lulalaby Lulalaby Apr 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume we decided against including the content inventory type (it can be received) as reference?

Copy link

@Icebluewolf Icebluewolf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wanted to point out a few things I saw :)
Sorry, if these are overly nit-picky.

Copy link
Contributor

@Lulalaby Lulalaby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, since it came up again on our lib server: It should be mentioned that components (namely text display) can mention users and roles.

Copy link
Contributor

@colinloretz colinloretz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇

@anthonydiscord anthonydiscord merged commit af1843d into main Apr 22, 2025
4 checks passed
@anthonydiscord anthonydiscord deleted the components-v2 branch April 22, 2025 20:10
Copy link
Contributor

@JustinBeckwith JustinBeckwith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"just let me sneak one more merge conflict in first"

@JustinBeckwith
Copy link
Contributor

dammit it too late

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.

9 participants