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

Rename link 'template' data attribute #83

Merged
merged 2 commits into from
Sep 13, 2012
Merged

Rename link 'template' data attribute #83

merged 2 commits into from
Sep 13, 2012

Conversation

gdlx
Copy link
Contributor

@gdlx gdlx commented Aug 29, 2012

Here is a fix for issue #80.

Link's template data attribute is already used by Twitter Bootstrap's tooltip JS, producing strange behavior when using both BS Tooltip and Cocoon on the same link.

This fix renames template to association-insertion-template to limit collision risk.

Since this attribute is generated by the gem itself, there should not be any regression, except if some user does JS customization of template, which is contradictory with the purpose of the gem.

Specs are updated with the new attribute name (I had to change data attributes order since Rails seems to sort them). All 30 specs successful.

I did not update the readme since there was no mention of template but maybe you will want to add a note about the change.

Thanks !

@nathanvda
Copy link
Owner

Nice one. Indeed the template name is obviously a bit too generic. Thanks.

nathanvda added a commit that referenced this pull request Sep 13, 2012
Rename link 'template' data attribute
@nathanvda nathanvda merged commit c2a0be5 into nathanvda:master Sep 13, 2012
@gdlx
Copy link
Contributor Author

gdlx commented Sep 13, 2012

Indeed the template name is obviously a bit too generic

Sure...but it's also true for Bootstrap...

I should send them an issue about that too, but as i'm using bootstrap-sass and not the root version, I don't feel comfortable sending a pull request.

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