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

Don't render any conditional content when block is empty #283

Merged
merged 2 commits into from
May 28, 2021

Conversation

peteryates
Copy link
Member

@peteryates peteryates commented May 28, 2021

When either no block or an empty block (i.e., one that fails Rails' #presence check) is provided in the block passed to a radio button or check box no conditional content shouldn't be rendered and the data-attribute that's used to toggle it should be omitted.

This tweak moves the logic to earlier in the object's initialisation so that both the @conditional_content and @conditional_id will not be set when the block is blank.

Closes #281, #282

Do not display a div if the content is blank
@peteryates peteryates force-pushed the empty-wrap-conditional-content branch from bacea62 to 0476f09 Compare May 28, 2021 08:52
@@ -181,6 +181,20 @@
end
end

context 'when an empty block is given' do
subject { builder.send(*args) { "" } }
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

When either no block or an empty block (i.e., one that fails
Rails' #presence check) is provided in the block passed to a radio
button or check box no conditioal content shouldn be rendered and the
data-attribute that's used to toggle it should be omitted.

This tweak moves the logic to earlier in the object's initialisation so
that both the @conditional_content and @conditional_id will not be set
when the block is blank.

Fixes #281, 282
@peteryates peteryates force-pushed the empty-wrap-conditional-content branch from 0476f09 to 810ddbd Compare May 28, 2021 09:10
Copy link
Collaborator

@jsugarman jsugarman left a comment

Choose a reason for hiding this comment

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

👍 Many thanks

@jrmhaig
Copy link
Contributor

jrmhaig commented May 28, 2021

Thanks, this looks great. I'll close #281

@peteryates peteryates merged commit bf4ea3c into master May 28, 2021
@peteryates peteryates deleted the empty-wrap-conditional-content branch May 28, 2021 13:13
peteryates added a commit that referenced this pull request May 29, 2021
These two classes act in a very-similar manner, much of the
functionality is duplicate. As all fieldset items are conditional, we
can safely remove the Traits::Conditional and just treat everything as
a FieldsetItem.

This is the same approach taken by the text fields and Traits::Input.

Refs #283
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.

4 participants