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

Adds Beaver Builder support. #33

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Pross
Copy link

@Pross Pross commented Dec 24, 2019

Fixes #32

@timbocode timbocode added this to the Version 1.0.0-Beta2 milestone Feb 5, 2020
@@ -0,0 +1,20 @@
(function($){
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation is a bit off in this file, which makes it harder to read. It looks like most other JS files in this project use tabs.

ClassicSEOIntegration.prototype.hooks = function() {
classicSEOApp.registerPlugin( 'bb-seo' )
wp.hooks.addFilter( 'cpseo_content', 'bb-seo', function(content) {
return window.classicSEO.beaverbuilder.pagedata;
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like window.classicSEO.beaverbuilder will be undefined if Beaver Builder is not loaded. Maybe this is not actually a problem though, since the script will only be registered if BB is loaded...

Copy link
Contributor

@timbocode timbocode Feb 9, 2020

Choose a reason for hiding this comment

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

Yes, the script won't load if BB is not installed and active.

*/
private function content_data() {

$id = $_GET['post'];
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like it needs a permission / nonce check.

Copy link
Contributor

Choose a reason for hiding this comment

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

For when this is revisited: using get_the_ID() might be a better option here?

Copy link
Author

Choose a reason for hiding this comment

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

get_the_ID() is a wp loop function. This is all happening in wp admin on a post/page edit screen.

@timbocode
Copy link
Contributor

Thanks James. I've carried out much testing on this code and as I've said to @Pross , I'm struggling to see what difference it makes to Classic SEO.

Without this code, Classic SEO does recognise a lot of the Beaver Builder content but, for example, it doesn't read alt tags on images, even with this code.

I'm planning to revisit this shortly as I'd like to get it into beta 2 but I've asked @Pross to take another look.

I may leave this until after v1 has been released.

@timbocode timbocode removed this from the Version 1.0.0-Beta2 milestone Feb 9, 2020
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.

Add Beaver Builder Page Builder support
3 participants