search

processwire / processwire-requests

ProcessWire feature requests.
40 stars 0 forks source link

Icon not visible as property of Template object #281

Open ghost opened 5 years ago

ghost commented 5 years ago

@Toutouwai commented on Oct 31, 2018, 12:42 AM UTC:

Short description of the issue

Field and Template classes have similar getIcon() and setIcon() methods, and the unprefixed icon value is accessible for both Field and Template objects as ->icon.

But for Field objects icon is visible as a property of the object whereas for Template objects it is not visible. So for consistency and transparency it would be good if icon was made visible in Template objects.

Optional: Suggestion for a possible fix

Set and get an icon property similar to the way it is done in the Field class: https://github.com/processwire/processwire/blob/0f9eb0aaf5219ad09eebe9b3e797575ea1ed51ad/wire/core/Field.php#L1267 https://github.com/processwire/processwire/blob/0f9eb0aaf5219ad09eebe9b3e797575ea1ed51ad/wire/core/Field.php#L1204

This issue was moved by netcarver from processwire/processwire-issues#740.

ghost commented 5 years ago

@adrianbj commented on Oct 31, 2018, 12:45 AM UTC:

+1 from me

ghost commented 5 years ago

@ryancramerdesign commented on Nov 30, 2018, 5:09 PM UTC:

I'm not sure that I understand. Can you describe what you mean by "icon is visible as a property of the object"? There shouldn't be any public properties on these objects. Or are you just talking about the phpdoc?

ghost commented 5 years ago

@Toutouwai commented on Nov 30, 2018, 11:13 PM UTC:

I mean that the icon associated with a Field and the icon associated with a Template serve a similar purpose, yet icon is a property of a Field object and not a property of a Template object.

Visible maybe wasn't the right word - maybe "discoverable" would be better. So if I dump a Field object I can learn that there is such a thing as an icon, what that icon property is or is not set to, and that I can get it with $f->icon. Whereas if I dump a Template object there is no clue about an icon at all.

Using var_dump instead of a Tracy dump in the screenshot below just so everything would fit on the screen... 2018-12-01_120301

ghost commented 5 years ago

@Toutouwai commented on Nov 30, 2018, 11:25 PM UTC:

Also just noticed that it's a bit unexpected that when getting the icon property the prefix option is used in Field::getIcon(). I expected that you would get the same property that is shown in a dump.

2018-12-01_121929

ghost commented 5 years ago

@netcarver commented on Mar 5, 2019, 10:13 PM UTC:

ryancramerdesign Bumping for your consideration of further information.

ghost commented 5 years ago

@ryancramerdesign commented on Mar 8, 2019, 3:01 PM UTC:

netcarver Toutouwai I understand the request and it makes sense, though I don't currently have the bandwidth to get into very specific low-level things like this (results of var_dumps of different object instances using different classes), unless it's actually a bug or an error. My focus is on the quality and consistency of the public API, but when it comes to the lower level behind-the-scenes stuff, like in this case, I sometimes do things differently as PW has been developed over many years. And when it comes to this repo, I want to stay focus on fixing bugs. Though it's fine to move to the requests repo if you guys would like to keep this open, and hopefully can get another look at this in the future. Thanks.

ghost commented 5 years ago

@adrianbj commented on Mar 8, 2019, 3:15 PM UTC:

Just to add to the request for consistency - it's confusing that these return different things: image

ghost commented 5 years ago

@Toutouwai commented on Mar 8, 2019, 9:19 PM UTC:

No problem for this to be moved to requests. netcarver, can you do the movebot thing? Thanks.