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

Fixing Primer::Forms::ActsAsComponent::TemplateParams keyword args #1312

Merged

Conversation

danielnc
Copy link
Contributor

@danielnc danielnc commented Aug 16, 2022

When using in conjunction with BetterHtml gem, the template params for the struct are not properly identified.

#<struct Primer::Forms::ActsAsComponent::TemplateParams source={:source=>"<%= render(SpacingWrapper.new) do %>\n  <% inputs.each do |input| %>\n    <%= render(input.to_component) %>\n  <% end %>\n<% end %>\n<% if after_content? %>\n  <%= render_after_content %>\n<% end %>\n", :identifier=>"/danielnc/.rbenv/versions/3.1.2/gemsets/gemset-v1/gems/primer_view_components-0.0.90/lib/primer/forms/acts_as_component.rb", :type=>"text/html", :format=>"text/html"}, identifier=nil, type=nil, format=nil>

You can see that identifier, type and format are nil while the source has the entire hash that is expected to be keyword args for the component

With the fix, all keyword arguments work as expected and primer is able to be used with the better_html gem

I am not sure where/how to add tests and if I should add anything to the change log, help is greatly appreciated

@danielnc danielnc requested review from a team and camertron August 16, 2022 16:46
@changeset-bot
Copy link

changeset-bot bot commented Aug 16, 2022

🦋 Changeset detected

Latest commit: 7a5e0ed

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/view-components Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@camertron
Copy link
Contributor

Hey @danielnc thanks for the PR! I'm really surprised this works at all since I thought keyword_init was required to be able to use kwargs to init structs. What version of Ruby are you using?

@danielnc
Copy link
Contributor Author

@camertron we are on ruby 3.1.2p20

@camertron
Copy link
Contributor

camertron commented Aug 19, 2022

@danielnc hmm very weird! In any case, we should merge this asap. Can you add a changeset (npx changeset) and merge/rebase main?

@danielnc danielnc force-pushed the danielnc/fixing_template_params_keyword_arg branch from 75bbcd5 to 7a5e0ed Compare August 22, 2022 11:00
@danielnc
Copy link
Contributor Author

@camertron let me know if this is all that is needed

@camertron camertron merged commit ff515aa into primer:main Aug 22, 2022
@primer-css primer-css mentioned this pull request Aug 22, 2022
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