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 for #264, repeatable groups with a WYSIWYG editor #657

Closed
wants to merge 92 commits into from

Conversation

@johnsonpaul1014
Copy link
Contributor

commented Jun 3, 2016

I believe this fixes the problem by setting up a new editor when the new group row is added. It is also necessary to destroy and reinit all the editors when sorting.

jtsternberg and others added 30 commits Feb 29, 2016
Need to explicity load CMB2 class in include_cmb method for back-comp…
…at (to keep old versions from loading as well). Fixes #520
Implement CMB2_Ajax as a singleton.
CMB2_Ajax is already being used as a singleton, but doesn't enforce it which leads to the static `hooks()` function, so let's enforce it and be done with it.
Merge pull request #602 from jrfnl/feature/cmb2_ajax-is-singleton
Implement CMB2_Ajax as a singleton.
@alexmansfield

This comment has been minimized.

Copy link

commented Jun 8, 2016

It looks like custom WYSIWYG buttons (such as the one Gravity Forms adds) don't get attached after the first WYSIWYG field.

@johnsonpaul1014

This comment has been minimized.

Copy link
Contributor Author

commented Jun 9, 2016

That's strange. I have some custom buttons I have added to our editor on the site I needed to implement this change for, and they are added as new editors are added to the group. Are you using multiple editors in the same group row?

@alexmansfield

This comment has been minimized.

Copy link

commented Jun 10, 2016

I'm not sure if it happens for buttons that are added directly to the TinyMCE editor. I was noticing it for buttons that get added above the editor (next to the "Add Media" button). For example, the Column Shortcodes plugin attaches a button there (other plugins do as well, but that was the first free example I could think of).

@johnsonpaul1014

This comment has been minimized.

Copy link
Contributor Author

commented Jun 10, 2016

I did some investigation into this, and it looks like the plugin you are referring to uses the edButtons JavaScript array to add the buttons. It would probably be possible to clone this somehow for the new editors, but I think that should be left to someone else as I don't need that feature for my current project. The change is still a huge step forward, because at least you get a real editor and not a broken one when you add a group row.

@alexmansfield

This comment has been minimized.

Copy link

commented Jun 21, 2016

Right, I wasn't trying to put down your improvements at all. Your patch is a huge step forward. I just noticed the missing buttons when testing it out and figured this was probably the place to mention it. I'm sorry if this was the wrong place to mention it. Thanks for the effort you put into this, and keep up the good work!

@johnsonpaul1014

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2016

Thanks. I agree; this is definitely the place to mention it. Hopefully someone else can grab the baton from here.

@dash8x

This comment has been minimized.

Copy link

commented on includes/CMB2_Utils.php in e61c154 Jul 1, 2016

This breaks on Windows XAMPP localhost due to \ and / mismatch

This comment has been minimized.

Copy link
Member Author

replied Jul 1, 2016

Can you test how it works if you change line 226 (above) to:

$theme_root = self::normalize_path( get_theme_root() );

This comment has been minimized.

Copy link

replied Jul 1, 2016

$theme_root = self::normalize_path( get_theme_root() );

Yeah, this fixes it

This comment has been minimized.

Copy link
Member Author

replied Jul 1, 2016

Can you please provide me the $theme_root dir without the normalize_path? I would like to write some unit tests for WAMP directory issues.

This comment has been minimized.

Copy link

replied Jul 1, 2016

C:\xampp\htdocs\uligamu-school/wp-content/themes

This comment has been minimized.

Copy link
Member Author

replied Jul 1, 2016

Thank you, tests added: e7012a6

@webakimbo

This comment has been minimized.

Copy link

commented Jul 18, 2016

@johnsonpaul1014 would love to use your branch for a time-sensitive project but the build appears to be broken. Is the plugin still working on your end?

@johnsonpaul1014

This comment has been minimized.

Copy link
Contributor Author

commented Jul 19, 2016

I forked off master instead of trunk, so that was also an issue. I am going to make a new pull request off trunk to see if that works this morning.

Paul

Date: Sun, 17 Jul 2016 19:17:33 -0700
From: notifications@github.com
To: CMB2@noreply.github.com
CC: johnsonpaul1@msn.com; mention@noreply.github.com
Subject: Re: [WebDevStudios/CMB2] Fix for #264, repeatable groups with a WYSIWYG editor (#657)

@johnsonpaul1014 would love to use your branch for a time-sensitive project but the build appears to be broken. Is the plugin still working on your end?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@johnsonpaul1014

This comment has been minimized.

Copy link
Contributor Author

commented Jul 19, 2016

I have submitted the new pull request.

Paul

Date: Sun, 17 Jul 2016 19:17:33 -0700
From: notifications@github.com
To: CMB2@noreply.github.com
CC: johnsonpaul1@msn.com; mention@noreply.github.com
Subject: Re: [WebDevStudios/CMB2] Fix for #264, repeatable groups with a WYSIWYG editor (#657)

@johnsonpaul1014 would love to use your branch for a time-sensitive project but the build appears to be broken. Is the plugin still working on your end?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@webakimbo

This comment has been minimized.

Copy link

commented Jul 21, 2016

Thanks! Will keep an eye out for activity.

jtsternberg added a commit that referenced this pull request Aug 1, 2016
Update readme, and reference all PRs/issues:
Fixes #26, fixes #99, fixes #260, fixes #264, fixes #356,
fixes #431, fixes #462, fixes #657, fixes #693
@tw2113

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2016

@johnsonpaul1014 are we good to close this one, if you've issued a fresh pull request with updated details?

@johnsonpaul1014

This comment has been minimized.

Copy link
Contributor Author

commented Aug 2, 2016

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.