BP-legacy: split group single admin.php in separate template files

[Offereins]

The buddypress/groups/single/admin.php file handles all the separate group admin (sub) pages. With the latest inclusion of the cover image page, this has now amounted to a file of almost 500(!) lines. Please can we split this into smaller pieces of separate template files, so that overriding these in a theme gives less concerns with BP updates.

[boonebgorges]

Seems like a good idea to me. Maybe put them in an admin or manage subfolder.

[Offereins]

Split /groups/single/admin.php into separate template files

[Offereins]

Well, here you have it, directly split into /groups/single/admin/ files :)

[DJPaul]

How about we roll this approach into BP-Nouveau?

[hnla]

We have rolled this into the new Template Pack already as part of a review of all templates in respect of simpler includes and re-usuable code snippets.

As we have a patch here and the changes ought to be non destructive in terms of styles/layouts I'll check the patch over and look to commit it for legacy too in this instance.

[hnla]

-01 patch re-builds original patch taking into account recent heading changes so re-freshes & updates.

[hnla]

New patch re-builds on original.

Tested across all sub screens to be working on trunk install.

We can commit when we get one other confirmation all works ok!

[r-a-y]

02.patch changes the /groups/single/admin.php template so we do not have to do so many if conditional statements.

Instead, I've done this:

<?php if ( bp_is_group_admin_screen( bp_action_variable() ) ) : ?>

        <?php bp_get_template_part( 'groups/single/admin/' . bp_action_variable() ); ?>

<?php endif; ?>

In the same file, one thing that I haven't changed, but probably should is this:

<?php /* This is important, don't forget it */ ?>
        <input type="hidden" name="group-id" id="group-id" value="<?php bp_group_id(); ?>" />

I know this is probably a remnant from the older bp-default templates, but since we deem this hidden input field important, we probably shouldn't leave it in the template and should hook it to either 'groups_custom_edit_steps' or 'bp_after_group_admin_content' instead.

It's not that big of deal if we do nothing here, since bp-nouveau is going to take over soon :)

[hnla]

@r-a-y That's a nice bit of simplifying to reduce to one template check line.

I've run with your suggestion to move the hidden input out and added it to groups-functions.php hooked on the after_content action. (not sure if that's best position etc etc).

.03 should be a complete patch (your admin.php changes were based on the earlier or an earlier version so was ?? hunked(2) so just updated my version manually for your changes.

If we can pass off or improve the hidden input change lets then commit and be done! :)

[hnla]

Updates -02 witt suggestion to move hidden input out of template to add_action in groups-functions.php

[dcavins]

This is so much easier to read when it's not monolithic, and will be much better for developers that need to override only one of the screens. Previously, it was a lot of work keeping up with the changes to admin.php if all you wanted to do was make a tiny change on one screen.

The indentation seems kind of arbitrary. Can the sub templates have no base indentation? (I'd be happy to refresh the patch if we're in agreement.)

Thanks for putting this in order! Let's get this committed so we can set about refreshing dependent patches.

[hnla]

Yes lets get this committed, we can spend too long back and fourth re-freshing patches ad nuaseam at times. :)

Not too concerned with indentation so yes if you feel it's better aligned to left to begin go for it ( in fact sure you're right there really).

Can we also confirm we're happy with the add_action move in -03, think it's ok but it was a suggestion so I made best stab at a position to move it to which may not be optimal.

[DJPaul]

👏

[dcavins]

Standardizes indents of .03 patch. Moves hidden input output action to bp-groups-actions.php.

[dcavins]

I've added a new patch that standardizes indents and spaces. I've moved the template action (the group ID hidden input outputter) to bp-group-actions.php which doesn't feel awesome, but I don't think it should go in functions because it's not really something that a dev would call. It's like we need a template-actions file. (But we don't really.)

I think it's ready to commit, once someone else has had a chance to sign off on it.

Thanks for this change; it's a really nice improvement for devs!

[hnla]

@dcavins Thanks, yep know what you mean with that hidden input it doesn't really live anywhere that makes sense.

[dcavins]

One more thought: These screens are wrapped completely in a form, which would make adding a search box in #6385 a bad/impossible idea. (Nested forms aren't allowed, and generally the closure of the inner form is interpreted as the outer form's closure because the inner form's opening tag is ignored.) We have two action points, bp_before_group_admin_content, and bp_after_group_admin_content, but both are contained within the form. Could we add an action point before the form, like bp_before_group_admin_settings_form , for items that need to exist on the screen but not be included in the form?

Otherwise, I guess we could move the <form> tags (and those inside-the-form action points) into every sub template, so those templates can add items outside the form, but that's so much duplication.

[dcavins]

Last chance to dance on this ticket. @hnla is going to commit this in about 18 hours, so please voice your objections now. :)

[r-a-y]

I've got no issues.

Would like a core dev to comment on what I mentioned in comment:12 about the hidden input field and moving that out of the template.

[dcavins]

Replying to r-a-y:

I've got no issues.

Would like a core dev to comment on what I mentioned in comment:12 about the hidden input field and moving that out of the template.

Hi @r-a-y- In ​https://buddypress.trac.wordpress.org/attachment/ticket/7079/7079.04.patch, I've moved that hidden input out to bp-groups-actions.php. Does that seem like a reasonable place to you for it to go?

I agree that it shouldn't be left in the template--your idea of adding it via an action is a really good one, in my opinion.

[r-a-y]

I've moved that hidden input out to bp-groups-actions.php. Does that seem like a reasonable place to you for it to go?

Oops, sorry. I glanced the last message of the thread without catching up! :)

04.patch looks good.

Maybe prefix the function with bp_. Starting the function with groups_ is an old relic from the v1.0-1.1 days I believe.

Hook could also go into bp-legacy's buddypress-functions.php. I just took a look at what bp-nouveau is doing and they are hardcoding the hidden input field directly:
​https://github.com/buddypress/next-template-packs/blob/afb886bde7f485e475d79a66afd23c9c490547fb/bp-templates/bp-nouveau/includes/groups/template-tags.php#L434

[dcavins]

@r-a-y: I like the idea of the hook going in the template pack. That makes sense, unlike what I had done before.

Attaching a new patch. Thanks for your feedback!

[dcavins]

Move hidden input output hook into bp-legacy/buddypress-functions.php

[hnla]

@r-a-y :)

I had moved the hidden input on your suggestion in my 02 patch comment #13

Function name was my bad and did think of locating it in buddypress-functions.php but thought better of but why I wanted your feedback on comment:12

All looks good though so lets commit!

[hnla]

In 11050:

Templates Legacy: Single Groups Admin Screens

Group admin template managing the settings & create steps sub screens has become a file which is hard for developers to navigate through to edit.

Commit re-factors the admin.php file to run as a primary include file checking for the current action variable to load the required template part.

Commit breaks out the sections representing settings /creation steps from the admin template to their own template include files located in a new directory 'single/admin/'.

Smaller split template files are now easier to manage & read for editing and overloading.

Commit re-locates admin files hidden form input to buddypress-functions.php hooking to the existing templates do_action function.

Thanks to Offereins for the original ticket - this commit follows closely the approach used in bp-nouveau templates.

Props offereins, dcavins, r-a-y

Fixes #7079

[hnla]

In 11051:

Templates Legacy: Single Groups Admin Screens

Add missed un-versioned new template include files for the admin screens.

Fixes #7079