Use WP page names for BP directory pages headings

[hnla]

Currently we set a hardcoded heading string for our main directory pages e.g Activity stream directory receives the title heading 'Site-Wide Activity'

Regardless of what we may set as the page name if we edit or create our own WP pages and assign in BP settings as pages to use BP will ignore the WP name re-setting the 'post_title' (bp-*-screens.php > directory_dummy_post() ) to the hardcoded string in buddypress()->{$component}->directory_title.

For users, and some developers this may be less than intuitive if they are used to WP pages name and slug being the strings used normally, and if wanting to change the BP strings they are left having to either filter ( admittedly easy enough to achieve for devs ) or having to do a text string translation, hard for many to grapple with.

Ticket proposes that we change this behaviour by addressing how we build bp_core_get_directory_title()

This function returns $title as set to buddypress()->{$component}->directory_title our dummy_post function then sets 'post_title' => bp_get_directory_title( 'activity' ),

Changing $title to bp_core_get_directory_pages()->{$component}->title will build the title string from the array BP builds of assigned or created pages.

In the simplest sense this change works as expected to change the title printed and the doc title element without apparent issues.

Issues and testing:

Need to establish when bp_core_get_directory_page_ids() is built as this array is required before we build out $pages if $pages isn't in the wp_cache.

If we have $pages populated at point where BP initially creates pages then it should be safe.

Need to test what happens on initialising BP on a new site.

Simply making this change may not be advised as we could change the headings on established sites unexpectedly and cause consternation with users, this is the usual problem with this sort of change, if we did this we may need something implemented such as a checkbox on the page settings in BP which if checked i.e 'true' would effect this change to using the page heading as set in the actual WP page else if false the established strings would continue to be used.

I'll push a patch up when I've done a little more testing.

I had thought there was a ticket already on this subject but couldn't locate one.

[hnla]

Initial patch as proof of concept only. Adds a new primary check on bp_core_get_directory_pages() before setup globals string check.

[hnla]

Initial '01' patch adds the check for bp_core_get_directory_pages() to the conditionals ahead of buddypress()->{$component}->directory_title

This works to use the WP Page name as found in wp_cache_get('directory_pages', 'bp_pages')if that cache is empty BP builds it from a wpdb query on posts. This feels as though it would be all that's required to set the display header title and wp_title, I've left the original series of checks in place so we fallback to using buddypress()->{$component}->directory_title as set in bp-{component}-loader.php setup_globals() args.

At this stage not tested is what occurs when we activate BP for the first time and create our initial BP dir WP Pages, but it reads to me as though there ought not to be problems here we'll create the pages before we have need to query for the page title string.

If this were simply included as is the consequence for existing sites would probably just be a change in the Activity component setup as this is the one component where we diverge from the page name we set in page creation and the name as set in setup_globals() 'Activity' Vs. 'Site-Wide Activity' display would change to 'Activity' unless the WP Page name had been changed. While not advocating we change sites display necessarily, if we were to and thinking through the process users would have to grapple with, at most, they would need to update their page titles for the BP pages, developers that had gone to lengths to change BP's default behaviour with ? filters would likely not be affected, filtering on bp_get_directory_title would override the changes made here.

[DJPaul]

Patch looks ok but it sounds like this needs some testing first.

[imath]

Hi @hnla

I agree it's a good idea. But i don't think we need to use bp_core_get_directory_pages() again as the buddypress()->pages global set by bp_core_set_uri_globals() is already using this function.

So we could probably use a patch looking like 6765-02c.patch. But if i look at the components loaders code it seems more consistent to do 6765-02a.patch as that's the way we do it for directory slugs.

6765-02c.patch seems a bit "agressive" with plugins components to me. In the same spirit we could also do 6765-02b.patch. Both are better for the user as he can set the title for any directory component and as a plugin developer i'd be ok with both.

But if we only want to deal with our components and leave the choice to plugins authors, then 6765-02a.patch is best :)

[hnla]

@imath choices, choices, choices :) ; I'm a little confused as to the best approach here now.

Your 02c patch is essentially the approach I took so tends to be the one I favour to go with and I thought at the time that I looked at this and this had to be the approach but maybe I imagine this.

I also tend to think the other two variations make sense, hence confused :)

In terms of end result all we need to achieve is that if the user re-titles their physical component dir page that this change is the one that overrules anything else, a simple principle that the other dir pages follow.

I'm inclined to go with your version of my patch so 02c but would prefer you make the final choice, I'll patch that choice and run some checks then commit as this is a fairly simple change that could do with closing out.

Thanks for adding these patches.

[imath]

you're welcome. I think i prefer 02a, because we mind our own business and do not decide for others ;)

[hnla]

I'll patch, test and commit .02a later.

[hnla]

Was about to commit, running a few further tests and the obvious? occurred to me:

This is in essence a breaking change.

It's mainly great whatever patch we run in terms of what will be achieved however...

For those few users that have simply activated BP and accepted defaults their site title would have been a hard coded string 'Site-Wide Activity' NOT! the WP page title.

Now with this change we will be fetching the WP title. On a new install or existing this will mean we display the actual page title.

As this use of page title - as with the other BP components - is important I propose pushing this through, and suffering the backlash! The solution for them is actually very simple to edit their page title back to what it was.

Feedback is sought!

[hnla]

As discussed in BP dev chat:
​https://wordpress.slack.com/archives/buddypress/p1462391810001293

An upgrade routine is considered wise for existing installs so that the page title ( post_title ) is updated to reflect the older hardcoded string Site-Wide Activity thereafter the ability to change by editing the actual WP page title is available, new installs will simply use the title we have always set Activity.

[boonebgorges]

6765-migration.diff​ is a suggested migration routine. It's extremely conservative: If the saved WP page title doesn't match the page title that BP sets during installation, we assume that the site admin has manually changed it, and we do nothing. Otherwise, we update the directory page titles as necessary.

It appears that on most installations (the possible exception being very old ones with 'Blogs' instead of 'Sites'?), the only thing that'll change is that 'Activity' will become 'Site-Wide Activity'. Is that correct? I must say that 'Site-Wide Activity' is a pretty terrible page title, and as a site owner, I'd be quite glad if it were to change overnight to 'Activity'.

[hnla]

@boonebgorges

Only read through this but stepping through and it seems to fit the bill perfectly, not sure why you're worried about being conservative, it's a simple or complicated as it needs to be, it does the job. There is only one situation where this may not be 100% ideal where a user changes the WP page title, sees nothing happen, shrugs and thinks no more about it, we come along see the change long forgotten about and effect the change for the user... to their surprise!

Lets commit, I can run some real world tests against an old VS. new install but can't see really where we would fail with that update routine.

[dcavins]

Uses @imath's 02a and @boonebgorges' migration and uses master page default titles list.

[hnla]

shame!

[r-a-y]

Just answered a forum post about this, so I'm posting that solution here if someone wants to use it temporarily:

​https://buddypress.org/support/topic/renaming-component-page-titles-and-breadcrumbs/

[DJPaul]

@boonebgorges how much of a proof of concept was your migration patch? Did it still need more work?

[hnla]

I'm re-instating this for 2.7 it's essentially a simple change whether it breaks installs or not, the fix for a few sites that would notice a change is to re-title their WP page, developers likely would have dealt with things in some other manner, Boone's migration worked for me in all scenarios I could think of, but Boone should be final judge of that as asked :)

[boonebgorges]

@hnla The approach in 6765-02a+migration+defaults.diff​ seems good to me. Can you do me a favor and refresh it for 2.7? If you want to commit it, go ahead. Or if you want me to take the blame, post the updated patch here and I'll do it :)

[hnla]

@boonebgorges I'll look at a re-fresh and a commit some point over the weekend.

[hnla]

In 11080:

Use WP page titles for BP directory pages headings

The activity directory heading historically used a hardcoded string for it's WP page title (post_title), with other component directories taking the name from the actual WP page title.

This commit brings all page headings for BP directories into parity, screens will look to the WP page(post_title) for the string to use, & BP sets a new series of initial default strings for new installs page creation.

An update routine is provided to ensure that no existing sites will suddenly see a new title string, if a previous install then update the post_title to older hardcoded strings (unless the page title changed by user), thereafter changes to the WP page title determine what BP will display.

Props boonebgorges, imath, dcavins, hnla
Fixes #6765

[espellcaste]

Good work!

I think bp_core_get_directory_page_default_titles() should be filterable! Don't you?!

[netweb]

There’s 3 instances where extra indentation tabs were added when they should should be only single spaces in:
​https://buddypress.trac.wordpress.org/changeset/11080/trunk/src/bp-blogs/classes/class-bp-blogs-component.php

[netweb]

Patch 6765.6.diff​ fixes the PHPDoc indentation

[hnla]

@netweb thanks I fixed earlier based on your Slack discussion.

@espellcaste If you think it's necessary, I tend to worry we end up throwing filters in to so many places that code gets harder and harder to modify without something breaking. If there is a use case and no doubt there is should we open a new ticket to cover that and perhaps ask @mercime nicely if we can squeeze it it in time?

[djpaul]

In 11085:

Core: rename directory page title migration function introduced in r11080

Our other version-specific update/migration functions aren't prefixed
with the release number, and instead rely on the PHPDoc block to
explain why and when they were introduced.

See #6765

[mercime]

@hnla if you can get it in before Beta 1 with ok from leads/other commiters :)

[hnla]

@mercime cheers.

Ticketed/patched on: #7259