-
Notifications
You must be signed in to change notification settings - Fork 24
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
fix(cf-field-radio): hide reset button if form is disabled #2635
fix(cf-field-radio): hide reset button if form is disabled #2635
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
The failing tests are the flaky ones mentioned here #2514 Feel free to merge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good apart from a minor nitpick. @MitanOmar could you fix that?
@@ -23,7 +23,7 @@ | |||
{{/if}} | |||
</label> | |||
{{/each}} | |||
{{#if (and @field.optional @field.answer.value)}} | |||
{{#if (and (and @field.optional @field.answer.value) (not @disabled))}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{{#if (and @field.optional @field.answer.value (not @disabled))}}
No need for the additional and
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i was searching in the documentation if and
accept more then two parameter, but i did not see it, all examples ware explaining with two options only, but it was in my mind, i will do it in new PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I was very convinced that I'd previously used it with more than 2 parameters but had to check the code to be 100% sure: https://github.com/jmurphyau/ember-truth-helpers/blob/master/packages/ember-truth-helpers/src/helpers/and.ts
The helper loops over all parameters and returns false
if any one of them is falsey.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I was very convinced that I'd previously used it with more than 2 parameters but had to check the code to be 100% sure: https://github.com/jmurphyau/ember-truth-helpers/blob/master/packages/ember-truth-helpers/src/helpers/and.ts
The helper loops over all parameters and returns false
if any one of them is falsey.
# [12.10.0](v12.9.0...v12.10.0) (2024-03-06) ### Bug Fixes * add optional peer dep on ember data ([033cb84](033cb84)), closes [/github.com/embroider-build/embroider/issues/1169#issuecomment-1081943925](https://github.com//github.com/embroider-build/embroider/issues/1169/issues/issuecomment-1081943925) * **cf-field-radio:** hide reset button if form is disabled ([#2635](#2635)) ([c96a8ae](c96a8ae)) * **cfb-slug-input:** correct scheduleOnce call ([0111c9e](0111c9e)) * **copy-form:** remove unnecessary attributes ([8d5cbb6](8d5cbb6)) * **copy-modal:** remove form id for proper slug validation ([fb8f806](fb8f806)) * incorporate review feedback ([8289bbd](8289bbd)) * remove flakey test ([#2642](#2642)) ([34859e1](34859e1)) ### Features * copy forms ([06ce87f](06ce87f)) * **form:** add method to refresh the answer of a field ([78fd9b5](78fd9b5)) * **form:** add widget override for number separator ([69419b4](69419b4)) * **form:** always fetch answer meta ([2255a80](2255a80))
🎉 This PR is included in version 12.10.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
No description provided.