Skip to:
Content

Opened 4 months ago

Closed 4 months ago

Last modified 4 months ago

#5299 closed defect (bug) (fixed)

BP-Default missing notifications templates?

Reported by: modemlooper Owned by: johnjamesjacoby
Milestone: 1.9.1 Priority: highest
Severity: blocker Version: 1.9
Component: Notifications Keywords: dev-feedback has-patch
Cc:

Description

Users reporting nothing displayed on notifications page. Not sure if component is using plugins.php but I fixed by creating notifications templates and adding a conditional in home.php

Attachments (2)

5299.01.patch (6.3 KB) - added by boonebgorges 4 months ago.
5299.02.patch (761 bytes) - added by boonebgorges 4 months ago.

Download all attachments as: .zip

Change History (7)

comment:1 boonebgorges4 months ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 1.9.1
  • Priority changed from normal to highest
  • Severity changed from normal to blocker

Ugh, I don't know how we missed this.

As things stand, users of bp-default or a child theme have the following options:

  1. Copy the bp-legacy templates into their themes and add a conditional in home.php (as modemlooper suggests)
  2. Disable the Notifications component, but then lose access to the notifications in the admin bar
  3. Do some sort of hack to prevent the Notifications nav item from being registered

1 and 3 are unacceptably difficult for most people, and 2 is a regression. We have to come up with a better solution.

  1. The obvious choice is to include the notifications templates in bp-default. See 5299.01.patch. I know that we're EOLing bp-default, but I think the understanding was that we'd do that *after* 1.9, not *with* 1.9. And in any case, this represents a pretty large breakage with the theme.
  1. Bail out of registering the Notifications nav item when theme compat is disabled and no template are found in the parent/child theme. See 5299.02.patch.

On balance, I think that (a) is the right way to go. Feedback welcome.

boonebgorges4 months ago

boonebgorges4 months ago

comment:2 johnjamesjacoby4 months ago

If it's causing problems, we can release a quick 1.9.1 with them in there. My understanding of our discussion was we were EOLing bp-default starting with 1.9, so I purposely did not include them.

Boone - which of your options is (a)? You used numbered lists.

comment:3 boonebgorges4 months ago

Oops, sorry. 1 is a. I think we should include the templates. See 5299.01.patch.

comment:4 johnjamesjacoby4 months ago

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

In 7688:

Notifications:

Add templates and parts for bp-default theme. Also modify home.php to include notifications.php.

Fixes #5299. Props boonebgorges. (1.9 branch)

comment:5 johnjamesjacoby4 months ago

In 7689:

Notifications:

Add templates and parts for bp-default theme. Also modify home.php to include notifications.php.

Fixes #5299. Props boonebgorges. (trunk)

Note: See TracTickets for help on using tickets.