Skip to content

Conversation

@mvkampen
Copy link

Keeping it uniform with Rails::Form. This gives more flexibility for arguments by default, while still allowing the attributes: keyword. Suports partial destructuring, thus preventing manual wrapping / unpacking of options.

[#54]

Keeping it uniform with Rails::Form. This gives more flexibility
for arguments by default, while still allowing the attributes: keyword.
Suports partial destructuring, thus preventing manual wrapping /
unpacking of options.

[rubymonolith#54]
@mvkampen
Copy link
Author

Jus trying to imagine whether this is a breaking change.
But since the method interface remains unchanged i think it is okay.
Since you will have to override those methods to call your custom components anyway, you can implement it to support their interface.

delegate :dom, to: :field

def initialize(field, attributes: {})
def initialize(field, **attributes)
Copy link
Author

@mvkampen mvkampen Apr 10, 2025

Choose a reason for hiding this comment

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

Inheriting on this class will break
Could keep support and merge or choose

def initialize(field, attributes: {}, **kwargs)
  @field = field
  @attributes = attributes.merge(kwargs)
  # or
  @attributes = attributes.empty? kwargs : attributes
end

@bradgessler
Copy link
Contributor

This would be a breaking change. The reason I made it a hash and not kwargs is because the attributes are intended to be used for the root tag in the component. That might be a reasonable assumption, but I'm not sure.

I could break it for the 0.6.x release.

@bradgessler
Copy link
Contributor

Found the reason I didn't pass attributes as a keyword splat.

https://github.com/beautifulruby/superform/blob/bde0dc61aa64ce32130a22e5f029655c585033f3/lib/superform/rails.rb#L87-L90

The collection keyword is not an HTML attribute, so I can't pass it directly into the initializer.

@bradgessler bradgessler added this to the 0.6.x milestone Aug 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants