WebDevStudios / WDS-Simple-Page-Builder

Uses existing template parts in the currently-active theme to build a customized page with rearrangeable elements.
GNU General Public License v2.0
136 stars 25 forks source link

Parts not being loaded into layout #16

Closed lmartins closed 9 years ago

lmartins commented 9 years ago

I've been trying to implement this plugin in one of my sites but im struggling a bit to get it working.

Im registering a layout like this:

register_page_builder_layout( 'events-layout', array(
     'part-events-calendar',
     'parts/events/part-events-loop'
), true );

And calling it from a page template like this:

{% do action('wds_page_builder_load_parts', 'events-layout') %}

Note this is twigs syntax for:

do_action( 'wds_page_builder_load_parts' , 'events-layout')

In the dashboard SPB seems to be aware of my registered layout: https://www.dropbox.com/s/twtmw8icd5tmr3x/Screenshot%202015-07-24%2019.54.06.png?dl=0

But the parts names doesn't get loaded in those select inputs, nor do I see any option on my page related to this layout.

Am I missing something?

jazzsequence commented 9 years ago

if you're using register_page_builder_layout, you don't add the full part path, you'd do:

register_page_builder_layout( 'events-layout', array(
     'events-calendar',
     'events-loop',
) );

...and that is assuming that both those template parts are in the same directory. Currently the plugin doesn't support parts in different directories (though that is something I want to try to figure out at some point).

after that, your do_action( 'wds_page_builder_load_parts', 'events-layout' ) hook should work.

lmartins commented 9 years ago

Hey Chris, thank you for your message. Im finally giving a got WDS after our conversation on your blog. When you say "assuming both templates are in the same directory" you mean in relation to the template that is calling them?

lmartins commented 9 years ago

Btw, I've just pulled everything to the root folder of this theme, refreshed a few times but still no luck. For some reason it doesn't pull the parts names for any of the selectors on the plugin.

jazzsequence commented 9 years ago

No, I mean PB assumes that all your template parts are in the directory that's set on the options page (default is /parts), so your theme directory structure might be like:

/css
/js
/inc
/images
/parts
index.php
functions.php
page.php
...etc...

Inside your /parts directory would be all your template parts. PB also expects them to have a prefix that matches the one set on the options page (default is part-, so you'd have:

/css
/js
/inc
/images
/parts
  part-events-calendar.php
  part-events-loop.php
index.php
functions.php
page.php
...etc...

does that make sense?

lmartins commented 9 years ago

Yes, makes total sense, and yeah I missed that folder part. After moving my files to the proper folder, I now have

parts/ part-events-calendar.php part-events-loop.php

Which match what is defined on the plugin settings.

When I get to the layouts portion though, they still don't give output anything on the selects: https://www.dropbox.com/s/l12894r2ibbwva6/Screenshot%202015-07-24%2020.18.05.png?dl=0

The layout is created with two template parts though. Will do some more testing to see if I can pinpoint what's failing.

jazzsequence commented 9 years ago

The template parts that show up in that dropdown should match whatever files are in the template parts directory (defined in the options) and with the parts prefix (defined in the options), so if your options were left at the default, and those two template parts are in the /parts directory, then you should have at least 2 items, Events Calendar and Events Loop.

lmartins commented 9 years ago

Chris, figured out what was going on, and its actually quite simple. I was under the impression SPB would support child-themes which seems not to be the case. I had my parts published on the parent theme, which for my workflow makes more sense. Once I put those files on the child theme they immediately popped in onto SPB settings. Maybe something that could be improved for future releases.

One aspect im still not sure im following. Individual part do now show up while editing pages. Still on SPB Global Parts settings this is what I see: https://www.dropbox.com/s/q331v17o48locik/Screenshot%202015-07-25%2010.01.45.png?dl=0

Shouldn't they have a select to pick from my configured parts?

Thanks again.

jazzsequence commented 9 years ago

Once I put those files on the child theme they immediately popped in onto SPB settings.

Yeah, I'm using get_stylesheet_directory instead of get_template_directory. I could, I suppose, use template directory, but it seems to make sense to use the child theme rather than the parent theme. I dunno, there wasn't a huge amount of hand-wringing over it. I could go either way, TBH.

Not sure what's up with the global parts. It could be something dumb like an instance where I'm using get_template_directory instead of get_stylesheet_directory. I can take a look when I get a chance, or PRs are always welcome. :)

lmartins commented 9 years ago

Considering that the default behaviour of get_template_part is looking for the parts with the following order:

wp-content/themes/twentytenchild/loop-index.php wp-content/themes/twentyten/loop-index.php wp-content/themes/twentytenchild/loop.php wp-content/themes/twentyten/loop.php

It would probably make sense the plugin to behave similarly. For my use case would be very useful.

jazzsequence commented 9 years ago

Yeah, but _s uses the /template-parts/ directory for all of those...

I guess how I feel about the parent/child theme thing is that if you're doing these sorts of customizations, probably you should be doing them in the child theme anyway and using Page Builder doesn't necessarily mean you aren't also using get_template_part. In the last couple projects we've used this plugin on, we've had a template part in the parts (or whatever) directory and inside those template parts used get_template_part to get the appropriate theme template part (which could be something like one of the ones you listed or could be something like content-single.php in the main theme folder). So you're giving the user the ability to add that component, but you aren't giving them control to break the theme (you wouldn't want the user being able to remove the Loop from a page, for example).

On the other hand, I do see a use case for adding support for different template part directories. In the ticket I created, #15, I was mostly thinking about it in terms of allowing plugins to have their own internal template part directory so all the template parts relating to a plugin could stay inside the plugin, but it could conceivably also apply to adding support for template parts in a parent theme. I don't think I'd ever recommend including the main theme folder of another theme, but possibly allowing a child theme to add the contents of the get_template_directory() . '/parts/ folder could be used there. I'm closing this issue but I'm open to keeping the discussion going. :)

lmartins commented 9 years ago

I see your point, and its pretty fair to say that I still need to get more experience on using SPB to be able to provide solid feedback. Thank you for you clarification here though. Cheers.

lmartins commented 9 years ago

Hi @jazzsequence,

I've noticed another problem, when using SPB in a child theme. As said above, for SPB to recognise my template parts I need to have them in the child-theme, but here's the catch, if the template that is calling those parts is on the parent theme, on the front-end those parts will still be loaded from the parent theme.

Perhaps a simple scheme will be useful to explain what's going on:

Case 1

Parent Theme
|_ archive.php (calling `wds_page_builder_load_parts` here)

Child Theme
|_ parts
   |_ part-part1.php
   |_ part-part1.php

With this setup, the parts names are loaded into the admin, nothing shows up on the front.

Case 2

Parent Theme
|_ archive.php (calling `wds_page_builder_load_parts` here)
|_ parts
   |_ part-part1.php
   |_ part-part1.php

Child Theme
|_ parts
   |_ part-part1.php
   |_ part-part1.php

In this scenario im repeating the parts both on the parent and child themes. They become effectively redundant but necessary. The parts name are loaded into the admin selector, the parts also are loaded into the front-end.

Now this is were things get confusing. If I add the archive.php to my child-theme and remove the parts from the parent-theme, the front-end will still be broken. The admin will continue to show the parts names, the front-end appears to be trying to load the parts from the parent-theme no matter what.

As it is makes it difficult for me to use SPB. I rely heavily on parent/child themes form my projects, and im guessing this will be a problem for things like Genesis Framework too. Any chance this can be improved?

jazzsequence commented 9 years ago

I'm not sure I understand why you would put the page builder action in the parent theme and not the child theme. Isn't the point of child themes to take a parent theme and add unique elements to it that override that of the parent? In that case, wouldn't you want to have the page builder action in the child theme, to use template parts that are in the child theme? Like I said before, I can totally understand wanting to support 2 separate directories but using page builder in the parent and not the child doesn't make sense to me.

Parent/child theme relationships work like CSS -- the thing that's declared last will always take precedence. If your parent theme has a header.php and your child theme has a header.php, the header.php in the child is used. It sounds to me like what you're trying to do is call something in the parent that actually exists in the child -- that's not something that would ever be possible (until multiple directories are supported for page builder parts). You wouldn't use get_header in the parent and try to load the header.php in the child.

lmartins commented 9 years ago

Hi Chris,

Sorry for all the trouble about this. I do work quite a lot with parent/child themes, I use a parent theme as a framework of sorts. But I can be using it incorrectly.

Here's my use case. Whenever I have a structure I use without changes on every or most sites, that structure (template file) goes for the parent theme. Let's say the archive.php file. In that scenario, I only override archive.php on my child theme if that site needs changes to the dom outputted by that template.

This I believe is WordPress's default behaviour for the template hierarchy, and it has been working really well for me. Now if you don't see fit for this in SPB it is totally fair, but I should also mention that it's not unfair to expect wds_page_builder_load_parts to behave similarly to how get_template_part does, and this is how get_template_part works:

{{ fn('get_template_part','parts/part-events-calendar') }} goes look always looks for the file on the child theme, if it doesn't find it there, it will then search for it on the parent theme.

wds_page_builder_load_parts on the other hand, seems to work the following way:

Parent Calling parts on parent Loads the parts from the parent theme, loads the parts names from the child theme

Parent calling parts on child Fails to load any parts.

Child calling parts on parent Works as expected on the front-end, the parts are loaded but the admin doesn't know about the parts names.

Child calling parts on child Works without any issue.

All scenarios above work with get_template_part, it always starts looking in the child theme and it only goes for the parent if the part is not found the child. That being said, I do realize that SPB also need to know about these files on the Dashboard, and there im not really aware how does the template hierarchy works.

This means that the only viable option is to repeat on the child theme every template that makes use of wds_page_builder_load_parts. Not ideal, but if not possible in any other way I guess will have to do it.

jazzsequence commented 9 years ago

it's not unfair to expect wds_page_builder_load_parts to behave similarly to how get_template_part does, and this is how get_template_part works: {{ fn('get_template_part','parts/part-events-calendar') }} goes look always looks for the file on the child theme, if it doesn't find it there, it will then search for it on the parent theme.

Yes, but Page Builder will always^ look in the folder that you defined as the one holding your template parts. So it's not exactly the same and it's not intended to be a direct replacement. :)

^ - _unless you've filtered the directory it's looking in. (documentation)

Parent Calling parts on parent Loads the parts from the parent theme, loads the parts names from the child theme

This needs to be addressed. I'm going to open a ticket for it. Parent should load parts from the parent.

Parent calling parts on child Fails to load any parts.

This is probably the way it's going to stay until multiple directory support is added. The parent would never know inherently that a child even exists, so it should never be looking at the child for template parts. (I understand that this might not be how get_template_part works...I could be swayed to check both directories if they are different...I'll need to take a look at how get_template_part actually works.)

Child calling parts on parent Works as expected on the front-end, the parts are loaded but the admin doesn't know about the parts names.

Will also need to be addressed with the parent loading parts from the parent.

Child calling parts on child Works without any issue.

Yay! :)

jazzsequence commented 9 years ago

Also, don't be afraid to call me on this stuff. :) I built this with very specific use cases in mind and obviously as it gets used more by more people, new use cases will crop up that we'll need to address. Supporting multiple directories (even just between a parent and child) adds a level of complexity that I hadn't anticipated. I'm not unwilling to change things, I just want to make sure when new functionality is added that we're __doing_it_right. :)

lmartins commented 9 years ago

Hi Chris, thank you so much for your feedback and willingness to help. With the two use cases you've mention need to be addressed I think it will be pretty much perfect in this regard. Looking forward for it.

Thanks again.

lmartins commented 9 years ago

Hi again Chris. Is there a technical reason for WDS requiring the parts to have a prefix such as part-? The reason im asking this is because im failing to load those parts using the regular get_template_part as they expect a specific structure to those file names.

jazzsequence commented 9 years ago

Just that part- is a fairly standard and generic prefix. But you can use whatever prefix you want by simply changing the setting on the options page. There's no reason you couldn't use this structure with get_template_part, however. If you had a template part here: parts/part-recent-posts.php with get_template_part, you'd use this: get_template_part( 'parts/part', 'recent-posts' ), so I'm not sure I understand the issue you're asking about.

jazzsequence commented 9 years ago

See also my comment here: https://github.com/WebDevStudios/WDS-Simple-Page-Builder/issues/21#issuecomment-128375139

You might be misunderstanding how to use/build template parts for Page Builder

lmartins commented 9 years ago

Thank you Chris. Yes, I was trying to load the parts as you described on your example. But lets forget about that, I was just looking for a workaround for the named layout not loading.