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

Return nil when image_picker is nil #149

Conversation

@westonganger
Copy link
Contributor

commented Jun 28, 2019

Previously when an image_picker attribute was nil or blank, it would still return the SectionImagePickerField object. This would cause any if statements to wrongly return truthy values.

did added a commit that referenced this pull request Jul 19, 2019

@did

This comment has been minimized.

Copy link
Member

commented Jul 19, 2019

In order to keep the code independent from the value itself, I took a different approach. I improved the {% if %} liquid tag in order to handle a syntax like this:

{% if section.settings.my_image is present %}
  <img src="{{ section.settings.my_image }}" />
{% else %}
  <p>No image</p>
{% endif %}

The nice thing about it is that we can also use it for other kind of drops.

@westonganger

This comment has been minimized.

Copy link
Contributor Author

commented Aug 1, 2019

Can we merge this, the code changes are so minor and makes it behave much more intuitively and as expected for the designer.

Your likely to keep seeing issues similar to #155 without this PR.

@did

This comment has been minimized.

Copy link
Member

commented Aug 1, 2019

@westonganger I understand but your PR doesn't cover the blank state. If for some reason, your image is an empty string, it will return '' which won't be considered as false in {% if section.settings.my_image %}.

That being said, you've got a good point, it's more intuitive, although I do like is present and that it solves other cases, like empty string testing for instance.

Anyway, I'm going to fix both your PR and the bug #155.

did added a commit that referenced this pull request Aug 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.