Skip to content

Conversation

@pierry01
Copy link
Contributor

@pierry01 pierry01 commented Nov 5, 2024

BEFORE
jean:fix-checkbox-group-before

AFTER
jean:fix-checkbox-group-after

@pierry01 pierry01 marked this pull request as ready for review November 5, 2024 18:15
this.#handleRequired();
}

#handleRequired() {
Copy link
Contributor Author

@pierry01 pierry01 Nov 5, 2024

Choose a reason for hiding this comment

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

it's necessary to run the same code in connect and onChange

rbui__form_field_target: "input",
rbui__checkbox_group_target: "checkbox",
action: "input->rbui--form-field#onInput invalid->rbui--form-field#onInvalid change->rbui--checkbox-group#onChange"
action: "change->rbui--checkbox-group#onChange change->rbui--form-field#onInput invalid->rbui--form-field#onInvalid"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the correct use of actions here is change, not input

change->rbui--checkbox-group#onChange needs to run before the validation of change->rbui--form-field#onInput and invalid->rbui--form-field#onInvalid

data: {
rbui__form_field_target: "input",
action: "input->rbui--form-field#onInput invalid->rbui--form-field#onInvalid"
action: "change->rbui--form-field#onInput invalid->rbui--form-field#onInvalid"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

same situation... change instead of input for radio

#getValidationMessage() {
const input = this.inputTarget;
const defaultMessage = this.inputTarget.validationMessage;
let errorMessage;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

more understandable code here

@cirdes cirdes merged commit 518c9de into ruby-ui:main Nov 5, 2024
3 checks passed
@sethhorsley
Copy link
Member

closes #181

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.

3 participants