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

Boolean attributes ($-expression and dynamic) #382

Merged
merged 2 commits into from
Aug 29, 2023

Conversation

malthe
Copy link
Owner

@malthe malthe commented Aug 28, 2023

While handling boolean attributes (previously a configuration known as literal false – or rather the opposite of that) previously supported $-expressions such as checked="${some_condition}", this behavior was accidentally changed in 3.8.0.

This has now been fixed.

Additionally, boolean attributes are now respected for dynamic attributes.

This closes issue #381.

@malthe malthe force-pushed the issue-381-boolean-attributes branch from 09a39b0 to 8b3ddc2 Compare August 28, 2023 11:13
@malthe malthe self-assigned this Aug 28, 2023
@coveralls
Copy link

coveralls commented Aug 28, 2023

Pull Request Test Coverage Report for Build 6009710511

  • 22 of 22 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 87.062%

Totals Coverage Status
Change from base Build 5338584587: 0.1%
Covered Lines: 4025
Relevant Lines: 4521

💛 - Coveralls

@malthe malthe force-pushed the issue-381-boolean-attributes branch from 8b3ddc2 to 442933a Compare August 29, 2023 08:13
@malthe
Copy link
Owner Author

malthe commented Aug 29, 2023

@mcdonc this should work correctly now – even for cases like an empty set or other non-true value.

  1. For a boolean attribute, if an $-expression evaluates to a false value (i.e., "not "), no text is omitted.
  2. If the attribute value is the empty string, the attribute name is printed, otherwise the attribute is dropped.
  3. Dynamic attributes are simpler because they don't use interpolation.

I have added some tests to showcase the new behavior.

@malthe malthe force-pushed the issue-381-boolean-attributes branch from 442933a to d5db293 Compare August 29, 2023 08:15
1. For a boolean attribute, if an $-expression evaluates to a false value (i.e., "not <expression>"), no text is omitted.
2. If the attribute value is the empty string, the attribute name is printed, otherwise the attribute is dropped.
3. Dynamic attributes are easier, because they're not using interpolation.
@malthe malthe force-pushed the issue-381-boolean-attributes branch from d5db293 to 76ccbed Compare August 29, 2023 08:18
@malthe malthe merged commit 2ed3454 into master Aug 29, 2023
10 checks passed
@malthe malthe deleted the issue-381-boolean-attributes branch August 29, 2023 09:56
@mcdonc
Copy link
Contributor

mcdonc commented Aug 29, 2023

Thanks for this work!

I don't quite understand why <input type="input" checked="${''}" /> evaluates to <input type="input checked="checked"/>? Isn't the empty string logically false?

@malthe
Copy link
Owner Author

malthe commented Aug 29, 2023

@mcdonc it works for me if I add the following test case:

--- a/src/chameleon/tests/test_templates.py
+++ b/src/chameleon/tests/test_templates.py
@@ -579,6 +579,7 @@ class ZopePageTemplatesTest(RenderTestCase):
                 '<input type="input" checked="${True}" />',
                 '<input type="input" checked="${False}" />',
                 '<input type="input" checked="${[]}" />',
+                '<input type="input" checked="${\'\'}" />',
                 '<input type="input" checked="checked" tal:attributes="checked default" />',  # noqa: E501 line too long
             )),
             boolean_attributes={
@@ -600,6 +601,7 @@ class ZopePageTemplatesTest(RenderTestCase):
                 '<input type="input" checked="checked" />',
                 '<input type="input" />',
                 '<input type="input" />',
+                '<input type="input" />',
                 '<input type="input" checked="checked" />',
             )),
             "Output mismatch\n" + template.source

@mcdonc
Copy link
Contributor

mcdonc commented Aug 29, 2023

Oh geez, I forgot the backslashes when adding the same test. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants