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

Added 'id' attribute on group field #1108

Closed
wants to merge 1 commit into from
Closed

Conversation

amans2k
Copy link
Contributor

@amans2k amans2k commented Mar 14, 2018

Hello,

Today one of my clients reported an error on his site.
'Cannot read property match of undefined ...'
src - https://i.imgur.com/1660qlC.png

While debugging I found, there was some code in client theme js file
which was trying to get 'id' of 'postbox' class element
src - https://i.imgur.com/xbjbTnX.png

and the page where the error was occurring has cmb2 group field.

I checked the cmb2 trunk and there was no id attribute
https://i.imgur.com/NaukHUd.png

I have added the 'id' attribute in the php and the js file as well.
run the grunt.

Kindly have a look.

ref class 'postbox cmb-row cmb-repeatable-grouping'
@jtsternberg
Copy link
Member

I'm not sure I understand the implication that all CMB2 postbox elements should have IDs in order to compensate for a client theme js file?

@tw2113
Copy link
Contributor

tw2113 commented Mar 14, 2018

I am inclined to agree with Justin here, and am wondering what's available for the theme to use that could more accurately find just what it's intending, or do some better checks for a found value before trying to do more.

@amans2k
Copy link
Contributor Author

amans2k commented Mar 14, 2018

I completely agree with you guys

Themes should do proper checking before calling functions.

I have already given a patch code to the theme developers https://i.imgur.com/pTtIKIg.png

In WordPress, 'postbox' class is usually added to meta box wrapper https://i.imgur.com/TtFfaqe.png, and every meta box has 'id' attribute
And it is used in CMB2 group field as well without the 'id' attribute

This is a theme js conflict with CMB2 plugin. And there is a significant audience for both.

It's a small change in cmb2 that can prevent js error on their client sites.

@jtsternberg
Copy link
Member

I've merged this (the new working branch is develop). Thank you for your work here.

lipemat added a commit to lipemat/CMB2 that referenced this pull request Apr 9, 2019
* upstream/develop:
  Corrected link to facetwp-cmb2 in README.md (CMB2#1248)
  Implement attribute escaping per VIP standards/@GaryJones
  Update readme with more available 3rd-party resources
  Ensure date picker fields can have a default value. Fixes CMB2#1245
  Ignore some more directories in the distribution package.
  Do exclude composer.lock file from archive
  Don't exclude composer.json from the distribution.
  Add example for using the query_args field arg to override the get_terms args
  assignment alignment and space replacement with tabs
  phpdoc and comment touchups
  Try to make travis happy
  Update changelog
  Rename row variables in cmb.addAjaxRow as technically the emtpyRow is the one being passed around. Also pass the new row to the cmb2_add_row trigger
  use phpunit 5.7 in PHP 5.6
  Add develop suffix to the loader
  Add apigen to version control
  Prep 2.6.0 for release
  Oops, fix the fix (from bcec44e)
  Add props for CMB2#588. Closes CMB2#588
  Add props for CMB2#463. Closes CMB2#463
  Add props for CMB2#953. Closes CMB2#953
  Ditch the field args, and only pass in the field object to the new field before/after hooks, and clean up docs
  Remove unused variables/method attributes
  Ensure value passed to CMB2_Utils::filter_empty is always an array. Fixes CMB2#1026
  Add section for Creating/Running Tests
  Add magic getter for CMB2_Option class. Fixes CMB2#1052
  Add props for CMB2#1108. Closes CMB2#1108
  Update test to reflect postbox groups getting ids
  Update cmb-group-id functionality a bit
  Add props for CMB2#1216
  update github issue/PR templates
  Do not trigger tinyMCE editor save for the activeEditor. Prevents cursor jump in Gutenberg. Fixes CMB2#1202
  see CMB2#1214 - fix tests.
  see CMB2#1214 - better placement of the lib dep registration.
  fix CMB2#1214 - Making a field repeatable generates JS error because of missing sortable lib.
  Add props for CMB2#1212
  Added 'id' attribute on group field ref class 'postbox cmb-row cmb-repeatable-grouping'
  Before/After row hooks
  Document the new field_argument filters, remove superfluous parameters and and change the last parameter to `$this`.
  Introduce three new filters to filter field arguments.
  WP_DEVELOP_DIR != $test_root
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.

3 participants