Skip to:
Content

BuddyPress.org

Opened 9 years ago

Closed 9 years ago

#6572 closed enhancement (fixed)

Conversation replies don't immediately inherit the HTML of custom templates.

Reported by: charlesmabe's profile CharlesMabe Owned by: r-a-y's profile r-a-y
Milestone: 2.4 Priority: normal
Severity: normal Version: 1.7
Component: Messages Keywords: has-patch dev-feedback
Cc:

Description

There seems to be a problem with the way new replies in member conversations output their HTML. I'm not very experienced with programming or design-- I'm just doing what I can to get my site up and running. But this is most likely a bug, not just my problem.

I created an extra div wrapper around the message content in my custom, overriding template file-- my-theme/buddypress/members/single/messages/single.php-- in order to put it in a box alongside the avatar, as shown in the attached image. This custom div wrapper starts just before <div class="message-metadata"> and ends just after </div><!-- .message-content -->. I also enlarged the avatar thumbnail width and height. The styling looks fine.

However, when I send a new reply and the Ajax loads it (from the reply box below the messages list), its immediate styling is the same as how it was in the legacy template, as seen in the attached image. Only if I refresh the page does the reply display properly using the HTML in my custom template and corresponding CSS rules.

I only have the BuddyPress and bbPress plugins installed and activated. I have Advanced Ajax Page Loader and BuddyPress Default Data installed but not activated. WordPress and all plugins are up-to-date.

If you manage to find a solution soon, I would appreciate it if you would tell me so I can continue working on this custom theme instead of waiting for a proper BuddyPress update. My site depends on it. Thanks.

Attachments (2)

eb049272b1.png (97.4 KB) - added by CharlesMabe 9 years ago.
6572.01.patch (6.2 KB) - added by r-a-y 9 years ago.

Download all attachments as: .zip

Change History (12)

#1 @r-a-y
9 years ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Awaiting Review to 2.4
  • Priority changed from high to normal
  • Type changed from defect (bug) to enhancement
  • Version changed from 2.3.2 to 1.7

The problem is due to us hardcoding the markup for the AJAX message:
https://buddypress.trac.wordpress.org/browser/tags/2.3.2/src/bp-templates/bp-legacy/buddypress-functions.php#L1509

Which can be different from the /members/single/messages/single.php template.

I think we should introduce a new template called /members/single/messages/content-single.php or something like that so we can share the markup in both instances.

Please test the attached patch and you should be able to just alter /members/single/messages/content-single.php and that should apply for both AJAX and regular PMs.

Note: We might rename the new template to something else like 'single-content.php' later on.

@r-a-y
9 years ago

#2 @DJPaul
9 years ago

  • Keywords dev-feedback added

@hnla @mercime your opinions on this would be appreciated, specifically on name of template part. Want some pattern that makes sense for other bits in the future. Thanks

#3 @hnla
9 years ago

I would have to setup a test case to check this out but having run through changes with the template pack a while back and given the nature of our JS & Ajax making this sort of change is tricky and always likely to break, I think it's less likely we have a bug here as much as just have some fairly precise requirements in markup structure and tokens, however I may have missed some detail in the opening comment and I haven't looked at r-a-y's patch which suggests there was something to fix. I'll try and setup a testcase when I have a minute to see what's occurring.

Version 0, edited 9 years ago by hnla (next)

#4 @r-a-y
9 years ago

this will affect anyone that has overloaded templates so we'll need to be clear in announcing those template changes.

I do not think this will affect anyone too much. A savvy developer would have had to unhook how BP adds the single message via AJAX and run their own routine, so this wouldn't affect their changes.

Naming wise maybe 'single-message' - is not 'content-single' a little non descriptive'?

Yeah, I was having a hard time naming the new template part. Core WP themes use this convention for template parts used in the loop - content-POSTTYPE.php. Maybe content-single-message.php or content-message-single.php? Or to be more in line with what we already have, maybe messages-single.php since we have a template called messages-loop.php?

For comparison, bbPress uses this convention - loop-single-TYPE.php - for items in the loop. They use content-single-TYPE.php as a wrapper template:
https://bbpress.trac.wordpress.org/browser/tags/2.5.8/templates/default/bbpress/content-single-reply.php

Last edited 9 years ago by r-a-y (previous) (diff)

#5 @hnla
9 years ago

I do not think this will affect anyone too much. A savvy developer would have had to unhook how BP adds the single message via AJAX and run their own routine, so this wouldn't affect their changes.

That's fair enough then.

Think I would be inclined towards messages-single as being clear and succinct, to the point; but feel perhaps the content- prefix a little too WP post typish?

But if we feel it's better to fit with bbP/WP conventions then we better just make it content-single-message

Ultimately with the number of partials we have not too fussed as long as we're happy that it's suitable as an adopted convention.

#6 @r-a-y
9 years ago

Think I would be inclined towards messages-single as being clear and succinct, to the point; but feel perhaps the content- prefix a little too WP post typish?

Yeah, I tend to agree. I think we can roll with messages-single.php for bp-legacy and when we do roll out a new template pack, we can have a larger discussion about template part naming.

#7 @hnla
9 years ago

@r-a-y setup a quick test as I wanted to see issue in action and resolution, resolution is perfect in being straightforward, sorts out the Ajax issue but also gains in breaking up a template a little so easier to read and deal with (hopefully we'll be able to do this a little more in other templates) and removing the Ajax hardcoded call back it's a bit of a happy win win ticket really.

Seems we just need others opinion on naming and perhaps location - less an issue probably pointless sub foldering along the lines of /_incl/.

Does make me wonder what other Ajax reload issues if any could be resolved in a similar manner. The solution in having

Last edited 9 years ago by hnla (previous) (diff)

This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.


9 years ago

#10 @r-a-y
9 years ago

  • Owner set to r-a-y
  • Resolution set to fixed
  • Status changed from new to closed

In 10048:

bp-legacy: Introduce new template part - /messages/message.php.

This template is used in /messages/single.php during the message loop to
display each message and when a new message is created via AJAX.

This solves an issue where we were duplicating the message markup in two
different spots, which should make message template overrides easier.

Fixes #6572.

Note: See TracTickets for help on using tickets.