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

Make Field#options public, enabling access from templates #1513

Merged
merged 1 commit into from Feb 13, 2020

Conversation

pablobm
Copy link
Collaborator

@pablobm pablobm commented Jan 1, 2020

Over at #612, a fair point was made: if field options were public, they could be accessed from templates. This would enable users to customize fields without having to create new field types, which I find a bit more cumbersome.

An argument against this change would be that it increases the contact surface of field types, which can be an issue later if we change the interface of fields.

An argument for this change would be that the interface of fields is unlikely to change radically at this point. If it did, probably subclasses would have to change anyway too, possibly forcing more significant changes than that of how to access the options.

I'm not 100% sure about this one. I'd like to hear more opinions, use cases, etc. So here's a PR, which hopefully will bring some feedback.

@pablobm pablobm changed the title Accessible from templates; avoids having to create new field types Make Field#options public, enabling access from templates Jan 1, 2020
Copy link
Member

@nickcharlton nickcharlton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pro this change. It's something that's come up a few times and seems wise to do.

@nickcharlton nickcharlton added the fields new fields, displaying and editing data label Feb 12, 2020
@pablobm pablobm merged commit a118f77 into thoughtbot:master Feb 13, 2020
@pablobm pablobm deleted the public-options branch June 28, 2020 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fields new fields, displaying and editing data
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants