Skip to content

Call Bool before evaluating {enabled/visible}_when condition for labels#1769

Merged
corranwebster merged 2 commits into
mainfrom
fix-label-visible_when-empty-string
Feb 22, 2022
Merged

Call Bool before evaluating {enabled/visible}_when condition for labels#1769
corranwebster merged 2 commits into
mainfrom
fix-label-visible_when-empty-string

Conversation

@aaronayres35
Copy link
Copy Markdown
Contributor

@aaronayres35 aaronayres35 commented Nov 1, 2021

closes #1768

WIP label because the still needs a test. support was introduced in #1544, but that needs a test as well. Not sure if I will have the time to write a test on this, but wanted to open a PR with the simple fix now so it isn't forgetten. (previous comment about difficulty I had writing tests for Labels {enabled/visible}_when #1544 (comment))

Note that for "normal" enabled/visible when we have cond_value come up in an if statement:

if cond_value and (at_init or not editor_state):

In the code here we really do want to always pass a bool to setVisible or setEnabled.

Checklist

  • Add a news fragment if this PR is news-worthy for end users. (see docs/releases/README.rst)

@aaronayres35
Copy link
Copy Markdown
Contributor Author

@corranwebster this PR was labelled as a WIP because it does not contain a test but it is a trivial change that fixes the associated issue. I had trouble writing a test for Label {enabled/visible}_when previously (#1544 (comment)) and am not sure I'll have the time to figure that out now. Wanted to get your thoughts on if you think it is safe / worth it to throw this fix into the next release or if we should just hold off.

@aaronayres35 aaronayres35 changed the title [WIP] Call Bool before evaluating {enabled/visible}_when condition for labels Call Bool before evaluating {enabled/visible}_when condition for labels Feb 21, 2022
@corranwebster corranwebster changed the title [WIP] Call Bool before evaluating {enabled/visible}_when condition for labels Call Bool before evaluating {enabled/visible}_when condition for labels Feb 21, 2022
Copy link
Copy Markdown
Contributor

@corranwebster corranwebster left a comment

Choose a reason for hiding this comment

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

LGTM

@corranwebster corranwebster merged commit c8cf340 into main Feb 22, 2022
@corranwebster corranwebster deleted the fix-label-visible_when-empty-string branch February 22, 2022 11:52
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.

visible_when on Label doesn't implicitly take the boolean of the specified trait

2 participants