Add ListableOption enum and update Field class to use it#56
Add ListableOption enum and update Field class to use it#56tdwesten merged 3 commits intotdwesten:mainfrom
Conversation
tdwesten
left a comment
tdwesten
left a comment
There was a problem hiding this comment.
Looking good, but please process my feedback! Thank you for extending this add-on!
src/Enums/ListableOption.php
Outdated
| case True = 'true'; | ||
| case False = 'false'; | ||
|
|
||
| public function toArray() |
There was a problem hiding this comment.
You can skip this toArray function and use $option->value.
There was a problem hiding this comment.
Yes, I actually considered that, but in that case the returned value would be a string instead of a boolean. I’m not sure that would be entirely correct?
The same approach is used in other enums as well – for example, CollapseOption and BardInlineOption.
Also, if we change this, we’d need to update the tests too, e.g.:
expect($field->toArray()['field']['listable'])->toBe('true');
Please have another look and let me know what you think.
| 'instructions' => $this->instructions, | ||
| 'instructions_position' => $this->instructionsPosition, | ||
| 'listable' => $this->listable, | ||
| 'listable' => $this->listable->toArray(), |
There was a problem hiding this comment.
So you can use here $this->listable->value :)
|
Hi @kgirzadas, I have made some small updates to your code. Thank you very much for your contribution. |
2 participants
closes #55
@tdwesten Please review the following changes:
ListableOptionenum with three possible options: hidden, true, and false.hiddento align with Statamic’s default behavior.->listable()method without arguments is equivalent to ListableOption::true.Let me know if anything should be adjusted!