Skip to:
Content

Opened 8 years ago

Closed 8 years ago

#2743 closed defect (bug) (fixed)

Update handling of "front page displays" option for trunk

Reported by: DJPaul Owned by: DJPaul
Milestone: 1.5 Priority: normal
Severity: Version:
Component: Core Keywords:
Cc:

Description

In 1.2, "Activity Streams" is added to the "Front page displays" list on Settings > Reading with a couple of helper functions to massage the option value.

As trunk has implemented the components as pages, we now have duplicated Activity Streams entries. We also need a way of handling setting components to the front page, as the front page template is loaded before most of BuddyPress.

Attachments (4)

2743.patch (12.6 KB) - added by DJPaul 8 years ago.
2743-1
2743-2.patch (13.2 KB) - added by DJPaul 8 years ago.
2743-3.patch (12.6 KB) - added by DJPaul 8 years ago.
2743-4.patch (13.1 KB) - added by boonebgorges 8 years ago.

Download all attachments as: .zip

Change History (11)

@DJPaul
8 years ago

2743-1

#1 @DJPaul
8 years ago

Issue with patch so far is with these bits:

if ( bp_is_blogs_component() && !bp_is_blog_page() || bp_is_component_on_front_page( $bp->blogs->slug ) ) 

We need to check if $bp->blogs is set. This applies to all components, not just blogs. I've spoken about this with Boone on IRC and one of the options we have is to set current_component to the appropriate component if on the front page, and use bp_is_blogs_component().
Another is to add extra checks to the bp_is_x_component() set of functions to check for the front page without messing with current_component.

#2 @DJPaul
8 years ago

An argument against setting current_component is that we have not done that in the 1.2 branch's implementation of this feature, and I've never seen any bug reports about it.

@DJPaul
8 years ago

#3 @DJPaul
8 years ago

2743-2.patch deprecates a couple more functions, and I've changed behaviour (and name) of the "main" new function introduced to bp_is_component_front_page(). Previous version basically did is_front_page() as well as checked if the BP component was set on the homepage, the new version just does the latter, so it is used in conjunction with the former. This better utilises and follows the pattern of the existing template tag functions.

Patch also resolves the concern in the first comment on this ticket.

@DJPaul
8 years ago

#4 @DJPaul
8 years ago

2743-3.patch changes the behaviour; rather than edit each component's directory function, if we are on the front page and a BuddyPress component is set to display, manually add it in to the parsed URI.

#5 @boonebgorges
8 years ago

Great patch, Paul. Like butter.

One problem. If you set Register as your front page, and you try to access the home page while logged in or while registration is turned off, you get a redirect loop (because of the checks at the beginning of bp_core_screen_signup() ).

I'm not sure how to handle this. We could filter the 'Front page displays dropdown so that you can't set Register as the front page (effective but unappealing). Or maybe a check at the beginning of bp_core_screen_signup() that checks for this situation and redirects to some page that is sure to be populated (see my half assed 2743-4.patch). What do you think?

Other than that, nice job with this one.

@boonebgorges
8 years ago

#6 @DJPaul
8 years ago

Good catch with the redirect. I believe you've hit the nail on the head as the members component is the only BuddyPress guaranteed to be loaded.

#7 @djpaul
8 years ago

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

(In [3488]) Allow any component to be set to display on the front page. Fixes #2743.

Note: See TracTickets for help on using tickets.