awcodes / filament-table-repeater

A modified version of the Filament Forms Repeater to display it as a table.
MIT License
201 stars 43 forks source link

Fix Table Repeater for MorphToSelect #67

Closed kzrinski closed 1 year ago

kzrinski commented 1 year ago

MorphToSelect's isRequired field is protected and so the TableRepeater fails when trying to access this property.

This instead uses the public function isRequired() to retrieve this value.

awcodes commented 1 year ago

I'll have to double check this. I think it might introduce a regression. Iirc, there was a previous issue with checking the isRequired() method because some fields don't have it accessible that way.

kzrinski commented 1 year ago

I've just gone through and triple checked all Components under the the Filament Forms package, and each one that makes use of the isRequired field also has access to the isRequired() function.

Most of them have access to this via extending the Field component which makes use of the CanBeValidated trait. Since the TableRepeater Component extends the Repeater Component which extends the Field component it has no issue accessing the protected isRequired property of other components which also extend Field. It's likely this issue hasn't been encountered as the MorphToSelect component is the odd one out not extending the Field Component.

I believe the solution proposed using the function is the more appropriate method of determining whether the TableRepeater is also required as it does not depend on Inheritance to access the value. If you would like I'm happy to refactor to check that the isRequired() method is 100% available instead of only checking for the property's existence.

awcodes commented 1 year ago

I remember now, Placeholder doesn't support isRequired(), that's what it was.

kzrinski commented 1 year ago

The Placeholder component doesn't have a isRequired field at all, which should mean that the changes presented do not affect the usage of it at all and it'll skip the if statement as it doesn't meet the conditions.

awcodes commented 1 year ago

Thanks.