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

Ease of use methods in custom and simple form builder #5

Merged
merged 3 commits into from
Jul 30, 2022

Conversation

Konicai
Copy link
Member

@Konicai Konicai commented Jun 8, 2022

  • Allows directly adding ButtonComponent to SimpleForm.Builder and FormImage to CustomForm.Builder
  • SimpleFormImpl.Builder was also missing some @Override annotations

This will allow for cleaner code when those objects have already been created from deserialization. Feel free to edit anything

* @since 1.1
*/
@This
Builder optionalButton(@NonNull ButtonComponent button, boolean shouldAdd);
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what the use of this method would be. If a button shouldn't be added then I'm not sure why the instance was created in the first place.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't have use for directly adding buttons anymore, but if I did, it is true I would instead check if it should be shown, and then if not, just add a blank component instead of doing some placeholder work (in order to get button IDs to lineup). I'll remove this

public Builder content(@NonNull String content) {
this.content = translate(Objects.requireNonNull(content, "content"));
return this;
}

@Override
public Builder button(@NonNull ButtonComponent button) {
buttons.add(button);
Copy link
Member

Choose a reason for hiding this comment

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

A null check is missing here

@Tim203 Tim203 changed the base branch from master to dev July 30, 2022 23:48
@Tim203 Tim203 merged commit b0dfa52 into GeyserMC:dev Jul 30, 2022
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.

None yet

2 participants