verbb / icon-picker

A Craft CMS field to select SVG or font icons from a folder for use in your content.
Other
17 stars 8 forks source link

`is empty` vs `isEmpty` #55

Closed robbeman closed 1 year ago

robbeman commented 1 year ago

Describe the bug

When checking if icons are set, I get inconsistent results using different checks.

For example, when no icon is selected:

icon is empty: {{ entry.icon|default is empty ? 'yes' : 'no' }}
icon.isEmpty: {{ entry.icon.isEmpty ? 'yes' : 'no' }}

Outputs:

icon is empty: no
icon.isEmpty: yes

getIsEmpty seems to work as expected, but it is to be deprecated?

Is there another reliable way to check for empty icons that I am not aware of?

I suppose entry.icon.type is empty would do the trick, but that doesn't seem what the type property is made for.

PS: the fact that this field returns a string instead of an object for "lingering" data is also messing with my head. See Entries retain field data that has been removed from entry type layout for more info. I am resorting to item.icon.type|default is not empty for now.

Steps to reproduce

  1. Create an icon field.
  2. Create an entry without filling in the icon.
  3. Try entry.icon is empty in your twig template.

Craft CMS version

Craft Pro 3.7.48

Plugin version

1.1.13

Multi-site?

Yes

Additional context

No response

engram-design commented 1 year ago

Wouldn't that be because you're calling entry.icon|default and then is empty on that? It otherwise seem unnecessary.

Those functions are only deprecated because they'll be removed in Icon Picker 2.

Calling {{ entry.icon }} will return a string representation of the IconModel (__toString()), but if you call it in functional tags, you'll be using the IconModel object. So we can do {% if entry.icon %} which under the hood would check for an empty object. You can also use {% if entry.icon | length %} which uses getLength(). And yes, you can also use {% if entry.icon.getIsEmpty() %}

So short version is that this will be addressed in Icon Picker 2.

robbeman commented 1 year ago

Hey, thank you for the reply. I think some other things added to the confusion and I felt like I was getting inconsistent results. In the meantime a small cleanup took place and and I found a more consistent way of dealing with this. Your comment also helped me understand things better.

Sorry for the bother and thanks again. Looking forward to v2. 👍