Skip to:
Content

BuddyPress.org

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#5549 closed defect (bug) (fixed)

Xprofile Visibility settings not visible on bp-default theme

Reported by: sbrajesh Owned by: boonebgorges
Milestone: 2.0.1 Priority: normal
Severity: normal Version: 2.0
Component: Settings Keywords: has-patch commit
Cc: sbrajesh

Description

In BuddyPress 2.0, xprofile visibility settings was moved to settings component as in ticket #5352

It works fine for themes using theme compat but does not work for bp-default or any other theme which contains members/single/settings/profile.php.

The problem is in bp-xprofile-screens.php in the function bp_xprofile_screen_settings() BuddyPress loads plugin template file instead of the members/single/settings/profile.php.

Here is a patch attached to fix the issue.

Attachments (3)

settings-profile-visibility.diff (486 bytes) - added by sbrajesh 6 years ago.
5549.02.patch (896 bytes) - added by r-a-y 6 years ago.
Attached patch only adds the "Settings > Profile" subnav item if the theme isn't bp-default
5549.03.patch (498 bytes) - added by boonebgorges 6 years ago.

Download all attachments as: .zip

Change History (11)

#1 @boonebgorges
6 years ago

  • Milestone changed from 2.0 to Awaiting Review

I haven't got a chance to test this at the moment, but won't this cause problems when bp-default doesn't have the /settings/profile.php template file?

#2 @sbrajesh
6 years ago

You are right about that. It will only work if the profile.php is present in settings directory of the theme. I updated it like this because BuddyPress had similar changes in BP 1.5 for the settings components and all the themes were forced to be updated.

#3 @r-a-y
6 years ago

  • Component changed from Core to Settings

We ran into a similar problem with the Notifications component in v1.9.

I'm torn because the proper "fix" would be to add full support for the "Settings > Profile" tab to bp-default. It wouldn't be too hard to do this, but are we adding new features to bp-default any more?

Since we do add a "Settings > Profile" item in the WP adminbar, it probably does make sense to support bp-default even if this is a new feature.

However, we do need to draw a line in the sand somewhere because we are planning on moving bp-default to the WP Themes directory and any new theme features after v2.0 will definitely not be added to bp-default.

I see two approaches:

  1. Add "Settings > Profile" page support to bp-default
  2. In the WP adminbar, only show the "Settings > Profile" subnav item for non-bp-default themes (using bp_use_theme_compat_with_current_theme()). This page is not a deal-breaker for bp-default as you can still set the profile visibility from "Profile > Edit".

@r-a-y
6 years ago

Attached patch only adds the "Settings > Profile" subnav item if the theme isn't bp-default

#4 @boonebgorges
6 years ago

  • Milestone changed from Awaiting Review to 2.0.1

I'm a bit confused here. At first I thought that this was the same thing as notifications in 1.9. But looking back at the changelog, it appears that bp-default was in fact updated to have the proper templates. See r7960.

The underlying problem in 2.0 is that the bp_core_load_template() call in bp_xprofile_screen_settings() is incorrect. See 5549.03.patch.

As an aside: In the future, we should come to a better consensus about what does and what does not get added to bp-default. If we're going to deprecate, we should actually deprecate. There would've been a way to write this new feature so that it degraded gracefully in bp-default (sorta like 5549.02.patch, but maybe with a more targeted locate_template() check, so that theme authors could manually add support to old themes if they wish).

#5 @r-a-y
6 years ago

  • Keywords commit added

I'm a bit confused here. At first I thought that this was the same thing as notifications in 1.9. But looking back at the changelog, it appears that bp-default was in fact updated to have the proper templates. See r7960.

You're right. Sorry for confusing you! :)

sbrajesh's first patch is identical to yours. Let's commit it.

In the future, we should come to a better consensus about what does and what does not get added to bp-default.

For sure.

#6 @boonebgorges
6 years ago

sbrajesh's first patch is identical to yours.

D'oh. :)

#7 @boonebgorges
6 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 8291:

Correct path to settings/profile template

The new Profile subtab of Settings is broken in bp-default-style themes without
this proper template path.

Fixes #5549

Props sbrajesh

#8 @boonebgorges
6 years ago

In 8292:

Correct path to settings/profile template

The new Profile subtab of Settings is broken in bp-default-style themes without
this proper template path.

Fixes #5549

Props sbrajesh

Note: See TracTickets for help on using tickets.