Skip to:
Content

Opened 3 years ago

Closed 13 months ago

#3586 closed defect (bug) (wontfix)

Settings fallback 'members/single/settings.php' is never used

Reported by: r-a-y Owned by:
Milestone: Priority: normal
Severity: minor Version: 1.5
Component: Settings Keywords:
Cc: info@…

Description

Changeset [5134] introduced a new fallback file:
/members/single/settings.php

However, this settings file is never loaded because the applicable BP settings action functions point directly to the /members/single/settings/x.php files.

For example, http://buddypress.trac.wordpress.org/browser/trunk/bp-settings/bp-settings-actions.php#L112 should point to 'members/single/home'.

And the template file - 'members/single/settings/general.php' should just contain the body of the content instead of the entire template.

---

If the above logic is what was originally intended, let me know and a patch will arrive on your doorstep!

Attachments (2)

3586.01.patch (6.8 KB) - added by r-a-y 3 years ago.
3582.02.patch (7.5 KB) - added by r-a-y 3 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 boonebgorges3 years ago

  • Milestone changed from Awaiting Review to 1.5.1

Yes, it looks like this was incompletely done. IMO, the more abstract, the better for themers; that suggests mirroring the other components by putting the header/footer/nav elements in the settings.php file and making the individual templates in /members/single/settings/ as small as possible.

r-a-y3 years ago

r-a-y3 years ago

comment:2 r-a-y3 years ago

  • Keywords has-patch added

Templates could use a once-over as far as whitespace and do_action hooks are concerned.

I left the whitespace the way it was originally in the template and kind of deliberated over the hooks.

comment:3 boonebgorges3 years ago

  • Milestone changed from 1.5.1 to 1.6

I'm going to bump this to 1.6, so that we don't go changing template logic in the middle of a dev cycle.

comment:4 boonebgorges2 years ago

r-a-y - This one kinda got lost in the milestone. I think it's probably OK to go in, as long as it's backward compatible with existing custom themes. Can you think of scenarios where it would break?

comment:5 r-a-y2 years ago

Backpat is going to be an issue, especially for custom themes and BP Template Pack users.

For these instances, theme devs would have to modify the settings templates to strip everything but the content. I'm reluctant to put theme devs through another template change like we did from 1.2 to 1.5, but it might be necessary.

Can't think of a good solution at the moment without introducing new template files and deprecating the older ones.

Any other ideas?

comment:6 boonebgorges2 years ago

I think you're right, r-a-y.

Any other ideas?

The other option is to go ahead and remove the unused template, and remove references to it from members/single/home.

When we do more of an overhaul to BP theme compatibility, we can think about fixing this. In the meantime, I don't think it's worth it.

comment:7 DJPaul2 years ago

  • Milestone changed from 1.6 to Future Release

Agree. Punting to future release based on above comments.

comment:8 wdfee15 months ago

  • Cc info@… added

comment:9 r-a-y13 months ago

  • Keywords has-patch removed
  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from new to closed

This is addressed in bp-legacy (theme compat) for 1.7, but unfortuntely bp-default will have to live with this defect due to backwards-compatibility concerns.

Note: See TracTickets for help on using tickets.