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

Fix class name merging for table helper #67

Merged

Conversation

bibstha
Copy link
Collaborator

@bibstha bibstha commented Jun 7, 2024

Previously options.reverse_merge would mistakenly not pick the merged classes with the tw helper. This was because reverse_merge would continue picking the existing classes.

With the change, we pick the new merged classes.

Previously `options.reverse_merge` would mistakenly not pick the merged
classes with the `tw` helper. This was because `reverse_merge` would
continue picking the existing classes.

With the change, we pick the new merged classes.
@@ -1,6 +1,6 @@
module Components::TableHelper
def render_table(caption = nil, **options, &block)
content_tag :table, options.reverse_merge(
content_tag :table, options.merge(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since if options contains :class already, reverse_merge won't touch it, we need to do merge instead.

@aviflombaum aviflombaum merged commit c7ee983 into aviflombaum:main Jul 20, 2024
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