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

Add a filter to allow for adding arbitrary HTML attributes to a group… #582

Merged
merged 1 commit into from Aug 12, 2016

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Feb 4, 2016

… wrapper element.

jrfnl added a commit to jrfnl/cmb2-conditionals that referenced this pull request Feb 4, 2016
This presumes PR CMB2/CMB2#582 has been merged into CMB2.

Fixes jcchavezs#2.

The js side of things is working.

@todo Needs checking if any more filtering for saving needs to be done on the PHP side of things.
@jrfnl jrfnl force-pushed the feature/add_group_attributes_filter branch from 69fd3fe to 8b03f8a Compare March 1, 2016 00:59
@jrfnl jrfnl force-pushed the feature/add_group_attributes_filter branch from 8b03f8a to 5ccb1b8 Compare April 22, 2016 11:48
@jrfnl jrfnl mentioned this pull request Apr 22, 2016
@jrfnl jrfnl force-pushed the feature/add_group_attributes_filter branch from 5ccb1b8 to e0c7a19 Compare April 23, 2016 00:25
jrfnl added a commit to jrfnl/cmb2-conditionals that referenced this pull request Jun 7, 2016
This presumes PR CMB2/CMB2#582 has been merged into CMB2.

Fixes jcchavezs#2.

The js side of things is working.

@todo Needs checking if any more filtering for saving needs to be done on the PHP side of things.
@tw2113
Copy link
Contributor

tw2113 commented Aug 2, 2016

@jrfnl may not be a bad idea to put a space before the $more_attributes variable to prevent running attributes together. Not quite sure how browsers would handle it, but perhaps better safe than sorry? We should also add attribute escaping for security purposes.

I am curious about the use-cases that you see with this.

@jtsternberg Since this one defaults to an empty string, and is just html attributes, i don't see a huge amount of breakage potential.

@jtsternberg
Copy link
Member

I'm inclined to move the CMB2_Types::concat_attrs() method to CMB2_Utils, and use that generate the string of attributes, and have the filter take an array. BUT.. this filter feels a bit arbitrary to me. Is there any reason to do this instead of doing things in JS, which is likely where you'll be wanting to use the extra attributes anyway?

@jrfnl
Copy link
Contributor Author

jrfnl commented Aug 12, 2016

Is there any reason to do this instead of doing things in JS, which is likely where you'll be wanting to use the extra attributes anyway?

@jtsternberg This is for the CMB2 Conditionals plugin to allow for conditional groups. The attributes determining the conditions would need to be added for the js to be able to pick up on it.

@jtsternberg jtsternberg merged commit e0c7a19 into CMB2:trunk Aug 12, 2016
@jtsternberg
Copy link
Member

Thanks @jrfnl! Hopefully that helps w/ the CMB2 Conditionals plugin. I've updated the filter to use concat_attrs, which means the filter sends an array and requires one back. See the change here: 337c243

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.

None yet

3 participants