Bring profile fields back into template parts

[johnjamesjacoby]

With recent changes to XProfile to create a more robust field types API, it seems we've lost the ability for themes to own the output experience. If so, this is hugely problematic, as we've once again coupled theme-side output with unmodifiable core code.

Creating this ticket as one part enhancement and one part support request incase I'm missing something obvious.

[boonebgorges]

Can you be more specific about how you're looking to change the output experience?

The only thing that is output by the class which might potentially be modifiable by the theme is what is defined in edit_field_html() - that is, the markup displayed when a user is editing his profile. If that's what you mean, then it is possible to override it, by overriding the class:

function bp5799_override_datebox( $fields ) {
    $fields['datebox'] = 'BP5799_XProfile_Field_Type_Datebox';
    return $fields;
}
add_filter( 'bp_xprofile_get_field_types', 'bp5799_override_datebox' );

class BP5799_XProfile_Field_Type_Datebox extends BP_XProfile_Field_Type_Datebox {
    public function edit_field_html( $raw_properties = array() ) {
        // Generate your own HTML
    }
}

Obviously, this is not a very "themey" way of doing things, but it does show that it's possible.

Two ways we could make this content easier to modify:

• At the beginning of each edit_field_html(), look for a template part in the theme, and load it and bail if found.
• Put the markup generated in edit_field_html() into an output buffer and pass it through apply_filters() before spitting it out.

However, I would argue that it's not a great idea to do either of these things. While it should be (and is) possible to override our default markup, we should not encourage it. The syntax of form markup is very particular, both in terms of HTML standards and in terms of what will make our save logic work. We do not want it to be overly easy for themers (a class of people that includes lots of beginners) to mess this up.

What's being produced in edit_field_html() is, for the most part, just a <label> element and the corresponding input/select/textarea. In the vast majority of cases, any necessary customization to these fields can happen through JS or CSS. If there are some areas where the markup that BP creates for these form elements is insufficient or incomplete, we should fix it in core, or add more limited filters (eg, it might make it easier to use a library like Chosen if we had filters on some of the CSS selectors). Overriding the markup totally should be a last resort, and our current API sorta makes it a last resort.

One last brief thought: while technically, what you've described here is a "regression" from the old way of doing things (in the sense that you did, in fact, used to be able to override this markup in a template), I would say that it's not a regression in spirit. One of the biggest problems of our previous implementation was that it put all of the edit markup into a huge if/elseif/elseif block, which was reproduced in two or three places throughout BP. So replacing the markup through templates would have involved reproducing hundreds of lines of template code, spread out through several templates.

[DJPaul]

Boone said this much more eloquently than I could have. Boone's suggestion that we could add something like a filter on CSS classes (etc) sounds a reasonable enhancement, but I think that ought to already be possible via the get_edit_field_html_elements filter, but it would be interesting to hear back from John in case he's tried something like that and found it wasn't sufficient.

[johnjamesjacoby]

Can you be more specific about how you're looking to change the output experience?

Completely. A good example is using an existing mobile front-end framework like Framework7 or Ionic, to fit into the expectations about what mark-up is necessary to fit form fields into themes nicely. Our core markup should be flexible, and easy to extend in the place where markup extension occurs -- the theme.

We've worked hard to eliminate hard-coded mark-up in our core files to allow them to be as flexible as possible, and this is a step backwards. We moved markup from the theme where it belongs, into core files, and abstracted ideas into several nested (less easy to grok) methods that require complex theme-to-plugin inheritance across projects without adequate dependency management.

What we have now is a mix of procedural function calls (like bp_get_the_profile_field_is_required()) and direct class method calls (like get_edit_field_html_elements()) that likely should have procedural wrappers, which also hugely prohibits the move back to template parts for field markup.

Conceptually, modifying template output should only require template parts in a child theme. It should never require having several classes in functions.php.

[johnjamesjacoby]

Another good example of needless over-abstraction is edit_field_options_html(). There's no way to get the selected element or its properties separately; all of the checks are hardcoded into the markup loop methods now.

[boonebgorges]

johnjamesjacoby discussed this in a bit more detail at WCSF. I think the conclusion we reached (John, feel free to correct me if I'm wrong) is that:

(a) we should definitely allow plugin/theme authors to provide a template that will override the logic in edit_field_html() if found. This is my first bullet point here: ​https://buddypress.trac.wordpress.org/ticket/5799#comment:3
(b) In order to make (a) work, it may be necessary to build some new procedural wrappers so that template files will have access to the required data in the global namespace
(c) Depending on how clean all of the above is, we could consider moving the markup building logic itself into template parts that would actually be bundled with the theme (something like members/single/profile/edit-fields/textbox.php etc).

I think we should go ahead with (a) and (b) for now, and maybe wait a version to see if this is enough to cover the relative edge case of wanting to override these templates. If so, we'll leave it at that. Otherwise, we can do something like (c) in a future release.

[DJPaul]

I am putting this in FR only because I don't know if anyone is going to be able to commit to work on this for 2.2. If a patch appears, let's move it back.

[DJPaul]

I'm interested in working on this for one of the near future releases.