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

Afform - Add "clear" button to non-required radios #23413

Merged
merged 1 commit into from
Jul 28, 2022

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented May 9, 2022

Overview

Fixes dev/core#3450
Allows the value of radio buttons to be unset if the field is not required, consistent with the rest of the CiviCRM UI.

Before

No way to clear radio buttons.

After

image

@civibot
Copy link

civibot bot commented May 9, 2022

(Standard links)

@civibot civibot bot added the master label May 9, 2022
@artfulrobot
Copy link
Contributor

artfulrobot commented May 9, 2022

Thanks @colemanw

r-run done on 5.47.4 site (patch applied cleanly).

  • ✔ no default, not required: radios show both unselected. Click one, filter works. Click the other, same. Click the ✖ and they get reset to null. All good.
  • ✔ no default, required: radios show both unselected. Click one, filter works. Click the other, same. There is now no ✖. All good.
  • ⚠️ default 'yes', not required: radios show both unselected, but with the clear button also. Also, the clear button appears if I select Yes, but disappears if I select No.
  • ⚠️ default 'no', not required: radios show both unselected, but with the clear button also. Also, the clear button appears if I select Yes, but disappears if I select No.
  • ⚠️ default 'yes' or 'no, and required: radios show both unselected (but I think the value is passed to the filter). Correctly, there is no clear button.

EDIT: note that in my testing I have been using a boolean field; this may differ from your screenshot where options were used, since for options "false" would be correct since that would be a label/name (were it an option value) as opposed to meaning boolean false.

@eileenmcnaughton
Copy link
Contributor

@colemanw looks like @artfulrobot has given this pretty thoughtful review - please self-merge this when you get @artfulrobot's go ahead

@eileenmcnaughton
Copy link
Contributor

@colemanw did you have a chance to look

Fixes dev/core#3450
Allows the value of radio buttons to be unset if the field is not required,
consistent with the rest of the CiviCRM UI.
@colemanw
Copy link
Member Author

I fixed the issue with the button showing up for "Yes" but not "No". As you might guess it was because false == null so I had to do type checking there.
The other issue is not directly related to this PR so I'm going to fix it separately.

@colemanw colemanw merged commit f71239a into civicrm:master Jul 28, 2022
@colemanw colemanw deleted the radioSilence branch July 28, 2022 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants