Skip to:
Content

BuddyPress.org

Opened 4 months ago

Closed 3 weeks ago

Last modified 2 weeks ago

#8079 closed enhancement (fixed)

BP Site-wide Notices: To enable paragraphs in the body text of the notification.

Reported by: kristianngve Owned by: imath
Milestone: 5.0.0 Priority: normal
Severity: normal Version: 4.2.0
Component: Messages Keywords: has-patch commit
Cc:

Description

Currently, it’s not possible to separate the two paragraphs. This is due to the text going through the wp_filter_kses() function which is removing the <p> tags from the text thus turning two paragraghs into a single paragraph.

I am requesting an enhancement to change this to an alternative function: bp_messages_filter_kses()

This would allow a filter to accept the <p> in the site-wide notice content.

Thank you for allowing me to create this ticket on the trac.

Kristian

Attachments (1)

8079.patch (5.9 KB) - added by imath 3 months ago.

Download all attachments as: .zip

Change History (21)

#1 @Venutius
4 months ago

  • Component changed from Core to Messages

When I first looked at this I thought it was down to wp_filter_kses since I could see the message correctly formatted in the database however removing wp_filter_kses does not resolve the issue and the the message content has the <br> tags separating the content removed.

I've stripped out the filter on save and the filter on remove and still the ,br. tags are removed. This only leaves the sql save and select functions so currently I'm a bit stumped as to what is causing the paragraphs to be merged.

#2 @kristianngve
3 months ago

Hi @Venutius

Thanks for looking into this. I was wondering if there has been any development since?

If not, what do you think is the best way forward on resolving this? Do you think there can be others within the department that can also take a look?

Looking forward to hearing back

This ticket was mentioned in Slack in #buddypress by venutius. View the logs.


3 months ago

#4 @imath
3 months ago

  • Keywords reporter-feedback added

Hi,

It's not just about changing a filter imho, because if you do so, here's the result in BP Legacy
https://cldup.com/vTaLD7gewx.png

And here's the result in BP Nouveau
https://cldup.com/IyHnnDwHzo.png

But I think you are using BP Legacy and you are referring to this function bp_message_get_notices(), do you ?

#5 @kristianngve
3 months ago

Hi @imath, thanks for contributing here. I am not so sure to be honest, but I have a few screenshots that hopefully will let you know for sure.

This is it in my Admin Dashboard:

https://prntscr.com/nsxgia

This is the page to write admin notifications:

https://prntscr.com/nsxgme

This is it shown on a page using a do widget short-code (plugin: amr shortcode any widget)

https://prntscr.com/nsxgra

Thanks again, and looking forward to hearing back

#6 @imath
3 months ago

  • Keywords has-patch 2nd-opinion needs-testing added; reporter-feedback removed
  • Milestone changed from Awaiting Review to 5.0.0

Thanks for your feedback @kristianngve

So here's what I'm suggesting in 8079.patch:

  1. Use the notice template tags inside bp_message_get_notices() to output the subject and text of the notice.
  2. Remove extra the p tag containing the title and text as the text will be wpautop thanks to the bp_message_notice_text template tag.
  3. Adapt the Legacy styles to the new markup.
  4. This will let users do remove_filter( 'bp_message_notice_text', 'wp_filter_kses' ) and choose the kses filter they prefer (eg: add_filter( 'bp_message_notice_text', 'bp_messages_filter_kses' ).

NB: The problem in bp_message_get_notices() is that wp_filter_kses is not applied as a filter.

@imath
3 months ago

#7 @kristianngve
3 months ago

@imath, Ah, I see. so just to confirm, I replace the red with the suggested green background code for the two templates?

If yes, I can have these two templates as child templates within my theme? Since the bp-custom.php file is in my plugins, what would be the root that'll work for these 2 customized templates? - with my functions.php file?

Last edited 3 months ago by kristianngve (previous) (diff)

#8 @imath
3 months ago

@kristianngve well not really :)

There's a core file into the patch. So if you want to test the patch, here's how you can do it :

  1. Install a server locally -> https://make.wordpress.org/core/handbook/installing-a-local-server/
  2. Install WordPress locally -> https://make.wordpress.org/core/handbook/tutorials/installing-wordpress-locally/
  3. Checkout BuddyPress trunk into /path/to/your/local/wordpress/wp-content/plugins/buddypress
  • using SVN : open a terminal and run this command :
    svn co https://buddypress.svn.wordpress.org/trunk/  /path/to/your/local/wordpress/wp-content/plugins/buddypress
    
  • using Git : open a terminal and run this command :
    git clone git://buddypress.git.wordpress.org/ /path/to/your/local/wordpress/wp-content/plugins/buddypress
    
  1. Download the 8079.patch
  2. run this command to move to the buddypress directory of your local install
    cd /path/to/your/local/wordpress/wp-content/plugins/buddypress
    
  3. Once there, apply the downloaded patch using this command
    patch -p0 < /path/to/where/you/downloaded/8079.patch
    
  4. Test :)

You can also read this WordPress documentation about working with patches, it's quite the same for BuddyPress.

#9 @kristianngve
3 months ago

Thank you so much for this @imath

I will work on this now, and I'll get back to you here if I run into some trouble. Hopefully not :)

#10 @imath
7 weeks ago

Hi @kristianngve,

Have you been able to test the patch?

This ticket was mentioned in Slack in #buddypress by imath. View the logs.


4 weeks ago

#12 @imath
4 weeks ago

  • Keywords commit added; 2nd-opinion needs-testing removed

@kristianngve

FYI I'm going to commit the patch so that it will be easier for you to test it once BP 5.0.0-beta1 will be published. This ticket will be set as resolved, but when testing the beta the issue is still there for you don't hesitate to reopen this ticket.

#13 @imath
4 weeks ago

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

In 12415:

Site Wide Notices: improve layout and possible customizations

By using existing BP Notices template tags into the bp_message_get_notices() function we are:

  • improving the way notices containing line feeds are displayed,
  • improving customization possibilities using the template tags filters.

Props Venutius, kristianngve

Fixes #8079

#14 @kristianngve
3 weeks ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Hi Imath, thanks for looking after and monitoring this case for me while I was gone.

I just got back from holiday recently and revisiting all unfinished things...

I have just downloaded the local wordpress site and ready to finish stage 3:

using SVN : open a terminal and run this command.

It's got very late so I will finish off these task tomorrow afternoon.

I'll keep you posted

kristan

#15 @imath
3 weeks ago

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

Sure, but please, reopen the ticket if you're sure the situation didn't evolve. For me it's fixed: it's possible to have paragraph now that I've committed r12415.

So I'm going to close it again as fixed :)

If you still have issues once you will be set, feel free to reopen ;)

Last edited 3 weeks ago by imath (previous) (diff)

#16 @kristianngve
2 weeks ago

Wow, sorry, I didn't realize earlier, and thank you, that's great work!!

So, how do I apply the fix to a live site? I see your link?

Is it as simple as uploading the patch to a specific file path in my cpanel (in buddypress)?

#17 @imath
2 weeks ago

Hi @kristianngve

No problem! The fix has been included into the development version of BuddyPress: what we call "trunk". So you don't need to apply the patch anymore, you just need to test the trunk version of BuddyPress. You can do it using the point 3 of this comment:

  1. Checkout BuddyPress trunk into /path/to/your/local/wordpress/wp-content/plugins/buddypress

Or you can do it using the download "Zip Archive" link of BuddyPress trunk at the bottom of this page

Or you can wait ~ 10 more days for the first beta release of BuddyPress 5.0.0 to be published on BuddyPress.org's blog.

#18 @kristianngve
2 weeks ago

Thanks for getting back to me @imath,

That's great news, so does that means it will be included in the next Buddypress update (if there are no issues with the beta release)?

Is that right?

#19 @imath
2 weeks ago

@kristianngve Absolutely 👌

#20 @kristianngve
2 weeks ago

Thank you @imath, I am happy to wait on it.

That brings this to a happy close. Thanks again for being awesome!!

Kristian

Note: See TracTickets for help on using tickets.