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

Added Support for Webhook Components #63

Conversation

jasonlessenich
Copy link

@jasonlessenich jasonlessenich commented Jul 24, 2022

This PR attempts to add Webhook Components. (Closes #46)
I've mainly just copied all changes from RohanGoyalDev's PR (#56) and fixed all the remaining issues.

Copy link
Owner

@MinnDevelopment MinnDevelopment left a comment

Choose a reason for hiding this comment

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

Looking good already, but will need some work.

In general the documentation should always include a description, even for getters. This is useful because it shows directly next to the method in the javadoc html.

You should check your file encoding to make sure my name actually shows up correctly, it is Spieß which includes a non-ascii letter.

You generally only add support for sending components, not getting them from existing messages via ReadonlyMessage. This is not a huge deal to me though, and I can just add that later myself. But I wanted to point that out regardless.

Comment on lines +196 to +199
* @throws java.lang.NullPointerException
* If provided with null
* @throws java.lang.IllegalStateException
* If more than {@value LayoutComponent#MAX_COMPONENTS} are added
Copy link
Owner

Choose a reason for hiding this comment

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

Throws docs should be after param and before return:

/*
 * description text ...
 *
 * @param  ...
 *         description
 *
 * @throws ...
 *         description
 *
 * @return ...
 */

.collect(Collectors.toList()));
mentions.getRoles().stream()
.map(Role::getId)
.collect(Collectors.toList()));
Copy link
Owner

Choose a reason for hiding this comment

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

Why was all this indentation changed?

void withDisabled(boolean disabled);

/**
* @return true if the button is disabled
Copy link
Owner

Choose a reason for hiding this comment

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

Missing description


/**
* Changes the disabled status of button
* @param disabled
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
* @param disabled
*
* @param disabled

*/

public interface Component extends JSONString {

Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change


public class Button implements ActionComponent, SingleEmojiContainer<Button> {

public static final int MAX_BUTTONS = 5;
Copy link
Owner

Choose a reason for hiding this comment

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

Move this into LayoutComponent

* @return true if the button is disabled
*/
boolean isDisabled();

Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change

* @see LayoutComponent#addComponent(ActionComponent)
*/
public interface ActionComponent extends Component {

Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change

jasonlessenich and others added 17 commits August 7, 2022 14:17
@MinnDevelopment
Copy link
Owner

Continuing this in #73

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.

Support for component types such as buttons and select menus
2 participants