Skip to:
Content

Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#6177 closed enhancement (fixed)

Improve representation of blog posts in the Activity Stream

Reported by: DJPaul Owned by: DJPaul
Milestone: 2.3 Priority: normal
Severity: normal Version:
Component: Activity Keywords: has-patch
Cc:

Description

When a Post is published, we create an Activity item for it and create an excerpt. If multiple images are in the post, we have some parsing logic that handles replacing the images with a single thumbnail image. The Activity Stream supports oEmbed and runs shortcodes, so those kinds of embedded content are richly embedded into the Activity Stream.

I believe we should iterate on the parsing logic to better extract media content from Posts, and use that to create better presented (and more interact-able) Activity items.

For an example, here’s a test site with BuddyPress 2.1.1 and WP’s theme unit test import data imported: http://www.stencilsplugin.com/activity/. The type of posts which I think BP should do a better job of presenting are demo’d here: https://imgur.com/a/f2b4T and https://imgur.com/VxQsXI9

Attachments (2)

6177.1.patch (62.3 KB) - added by DJPaul 3 years ago.
activity-comment.1.patch (898 bytes) - added by DJPaul 3 years ago.

Download all attachments as: .zip

Change History (38)

#1 @DJPaul
3 years ago

I've been working on this for a couple of months, and have the following changes which, along with the original suggestion, need review. I've been building these over in Github; please review the overall branch changes, rather than on a per-commit basis.

The "media-extractor" branch adds a new BP_Media_Extractor class which parses content and extracts info about media stored within.

The "media-extractor-activity-stream" branch changes the Activity Stream to use the media extractor class to generate the richer Activity items.

I could go into more detail about some of the architectural decisions but I hope getting your fresh eyes and interpretation will help generate meaningful feedback, and suggestions.

If anyone wants to contribute to this "patch", please submit pull requests to my Github branch (let me know if you do this because I tend to not notice I have incoming pull requests!). If this proposal is accepted and once all the code has been reviewed/iterated on, I'll then create a patch file, etc.

#2 @DJPaul
3 years ago

Here's a few screenshots (a couple of them are comparisions) but to see the affect, it's best to grab my github branch and test it on a dev site. https://imgur.com/a/npgae

Last edited 3 years ago by DJPaul (previous) (diff)

#3 @johnjamesjacoby
3 years ago

  • Keywords 2nd-opinion added
  • Milestone changed from Awaiting Review to 2.3

#4 @boonebgorges
3 years ago

!! This is great, DJPaul. Screenshots and code are super cool.

Just glancing it over, my only thought is about performance. Have you done any tests to see if there's a measurable impact to all this text processing every time the page is loaded? I wonder if it's worth investigating a caching layer around bp_activity_create_summary(), or around BP_Media_Extractor::extract().

#5 @DJPaul
3 years ago

Thanks for taking a quick look, Boone. I really appreciate it.

I had done some testing using xhprof to performance test the unit tests I wrote. That revealed some useful areas of improvements, such as avoiding regex calls where possible (I retested that assumption and found that it's still true). In terms of how bp_activity_create_summary is currently, it's only used for Posts on the transition_post_status hook, which means it isn't being run at real-time.

Please would someone remind me what the change was to activity objects, I think in one of the last couple of versions? One of the fields were hardcoded (so they couldn't be translated? or something) and then we changed it so they were dynamically generated, which I assume is what Boone is getting at here.

I will look into using bp_activity_create_summary on the activity objects with type = activity_update. I can't think of any other types of activity object that have a content item set that may be worth doing this one (maybe forums, but that's bbPress)

Last edited 3 years ago by DJPaul (previous) (diff)

#6 @DJPaul
3 years ago

  • Owner set to DJPaul
  • Status changed from new to accepted

#7 @DJPaul
3 years ago

Just updated these github branches against master.

#8 @rosyteddy
3 years ago

When we create Gallery by post-new.php > Add Media > Create Gallery
BP activity stream does not show any thumbnail, not even one. Since the post is all about pictures, ideally it should show one or two thumbnails ( or an asynchronous tile of N number of images)

Incidentally p2 theme does show gallery pictures thumbnails in its stream though it has no control on how many thumbnails should be shown.

Do I post this as a separate issue? Thanks for your help so much.

#9 @DJPaul
3 years ago

Hi @rosyteddy

First all of, thanks very much for contributing to this discussion. You're right that in current versions of BP, most of the time, we do not show images (from Posts, or attached as Galleries) in the activity stream. This is exactly one of the things that I hope we can improve with this ticket for BuddyPress 2.3.

The work I've done here so far means, for your example, that an image from a gallery in a post *will* be shown in the activity stream. However, it won't show multiple images, or a grid/tiles of the images in the gallery.

While I think that would be a really great enhancement, it needs to wait for a later release (perhaps even BP 2.4) due to some edge cases around involving making sure we're loading the gallery CSS + JS markup -- when appropriate -- on the Activity directory page. We're planning on some new templates for a BP release sometime this year, and that will be the right time to further iterate on this feature.

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


3 years ago

#11 @rosyteddy
3 years ago

Hi @DJPaul
Thanks.

"The work I've done here so far means, for your example, that an image from a gallery in a post *will* be shown in the activity stream."
Can you kindly point me to the file which if I replace with, can do this now? I am eager to test and implement :)

"While I think that would be a really great enhancement, it needs to wait for a later release (perhaps even BP 2.4) due to some edge cases around involving making sure we're loading the gallery CSS + JS markup"
Can this be reused from the p2 theme? This theme also adds "Add media" link to the Whats New box and uses core WP media. If this can be done, the media problem of buddypress also can just vanish. It seems all the code is there (in p2 and its responsive version Mercury) - only thing we need to integrate it in BP.
I know coding needs energy+time+motivation and probably +money. Sigh, I do not have coding skills and need fast implementations.
Thanks again for the fast response and help.

#12 @DJPaul
3 years ago

#6246 is kinda related.

#13 @DJPaul
3 years ago

I looked into using the new functions with activity_update activity items. It didn't work well due to edge cases around creating excerpts for these type of items and the complexity around getting excerpts implemented consistently, as well as other odss parts of the template like the one-line summary that appears at the top of user profiles (with BP-Default/Classic).

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


3 years ago

#15 @DJPaul
3 years ago

To bring the ticket up to date -- this is "done", I now need code review or testing or any other kind of feedback to move ahead.

#16 @rosyteddy
3 years ago

@DJPaul — Thanks a lot.
In the meanwhile I was observing that

short codes which are so wildly popular in wordpress can show up in unexpected ways in BP Activity Stream excerpts, for example Siteorigin-panels [siteorigin-panels.2.0.7 plugin] and some others can make the excerpts unusually long or broken.


images attached in comments and showing that as non-obtrusive thumbnails in BP Activity Stream is done nicely by Imaths's bpcheckin plugin but limited to only that plugin.


BP misses live updates to blogs by plugins like Live blog


My suggestions :
Maybe, BP should detect stuffs like shortcode and stuffs like siteorigin-panels and do not show any excerpt at all for such posts
Adoption of comment thumbnails (in comment excerpts) from that plugin

#17 follow-up: @DJPaul
3 years ago

In current releases of BuddyPress, shortcodes in blog posts are sometimes rendered in the activity stream (and sometimes not), depending on the length of the blog post content/excerpt.

The proposed change on this ticket unifies the handling of this, and it makes it so that shortcodes are stripped from the content and never rendered.

@DJPaul
3 years ago

#18 @DJPaul
3 years ago

I've added a patch for the proposed change. The code hasn't change from the Github branches, but I just wanted to get something up here in case this was making it harder for people to test it.

#19 @boonebgorges
3 years ago

DJPaul - Are there specific aspects you'd like feedback on? The general architecture looks really nice to me, and the feature set seems excellent. Are you hoping for line-by-line code review, or just another opinion on the strategy? I personally am confident enough in the approach that I'd be glad for you to commit it, and we can work out any kinks in trunk.

#20 @DJPaul
3 years ago

I guess I was hoping for any architecture feedback, mainly. If you think it's OK to commit for now, I might just do that.

#21 in reply to: ↑ 17 @r-a-y
3 years ago

The use of bitmasks in BP_Media_Extractor is interesting.

So if I wanted to grab only audio and video, I would do something like this?

$stuff = BP_Media_Extractor::extract( $text, BP_Media_Extractor::AUDIO | BP_Media_Extractor::VIDEO );

Neat!


I think bp_activity_create_summary() can be tweaked so we do not search for featured images and galleries unnecessarily. Featured images and galleries are really only WP things, so we should only attempt these types of extractions if the activity type is a blog post or custom post type.

If possible, I would love to move the extract_images_from_galleries() and extract_images_from_featured_images() class method logic to the Blogs component, while maintaining how the BP_Media_Extractor class works (in particular, the bitmasks). Though, I do see how that might be difficult to implement.

That being said, I think it's neat to have a media extractor in core! I can see a plugin adding some description to activity items such as This activity item has two images and one embedded video or something like that.

Nice work, Paul!

#22 @rosyteddy
3 years ago

Thanks a lot Devs

I forgot to mention when there's a really long piece of text posted as update, so that "Read more" is shown in the stream, the entire stuff opens in the stream but there is no "Read less" toggle. Thanks.

#23 @DJPaul
3 years ago

  • Keywords has-patch added; 2nd-opinion removed

I think bp_activity_create_summary() can be tweaked... Featured images and galleries are really only WP things

Agreed with featured images, but not galleries. The gallery shortcode could conceivably be used in any context.

bp_activity_create_summary() can be tweaked so we do not search for featured images and galleries... (they) are really only WP things, so we should only attempt these types of extractions if the activity type is a blog post or custom post type.

We already do. At the moment, we only use bp_activity_create_summary for blog posts and custom post types; see bp_activity_post_type_publish, bp_activity_post_type_update, and bp_blogs_record_activity.

If in future iterations (be it in later release or subsequent changes in 2.3) we use bp_activity_create_summary for more types of activity item (status updates, for example), then yes, appropriate adjustments, such as what you describe, would need to be made.

#24 @DJPaul
3 years ago

Also,

I would love to move the extract_images_from_galleries() and extract_images_from_featured_images() class method logic to the Blogs component

This is a good question and shows you took time to read the code carefully, so thank you. :)

Originally, I went to build the extractor as a static class, and galleries/featured images were in a child class ("BP_Media_Extractor_Post"). I had to give up on this due to inheritance issues with calling re-implemented methods (if we supported a later PHP version, Late Static Binding would have fixed this). It was also fiddly because then (we) would need to know when to use either one, and that's a bit fiddly. You can find some commits where I started re-factoring the code here, here, here, and here. This was when I hit upon the current implementation.

My biggest objection to moving the methods mentioned into the Blogs component, the potential complexity of that aside, is that I am not sure there is a good reason to require the Featured Image extractor to work only when the Blogs component is active. I see this as more of a general purpose library, like what we often seem to be building nowadays (suggestions, this, the attachments API).

#25 @djpaul
3 years ago

In 9619:

Core: add media extractor class.

Parses a block of text and extracts info about media contained within. Tests are included.

This was written to enhance the Activity Stream's generation of excerpts of blog posts, which will follow in a subsequent commit.
Inspired by a similar class from Jetpack.

See #6177

#26 @djpaul
3 years ago

In 9621:

Activity: improve blog post excerpts by showing rich media from the post.

Prior to this change, when we created excerpts for new/updated blog post activity items, we'd try to use any single image referenced in that blog post, and use it to decorate and bring interactivity to the activity stream item. This was inconsistently applied -- it was dependant on the length of the original post -- and ignored other possible media sources, such as oEmbeds and video/audio shortcodes.

Now, we use the media extractor (r9619) to extract comprehensive media information from a new/updated blog post, and use the information to build a much richer excerpt for blogs posts for activity items.

See #6177

#27 follow-up: @imath
3 years ago

DJPaul, there's a notice error when commenting a custom post type within the activity stream if 'Allow activity stream commenting on blog and forum posts' is on at line 414 of bp-activity-filters.php

#28 in reply to: ↑ 27 @DJPaul
3 years ago

Replying to imath:

DJPaul, there's a notice error when commenting a custom post type within the activity stream if 'Allow activity stream commenting on blog and forum posts' is on at line 414 of bp-activity-filters.php

How do I recreate? Register a custom post type, then add_post_type_support ?, publish a post, and then comment on the resulting activity item?

#29 @imath
3 years ago

Sorry, the fastest way to reproduce is :

  • Activate this option 'Allow activity stream commenting on blog and forum posts'.
  • in a bp-custom.php file, simply add : add_post_type_support( 'page', 'buddypress-activity' );
  • Post a page,
  • go to activity stream and comment the page activity and you'll get the warning.

#30 @imath
3 years ago

DJPaul, the warning is also appearing for regular posts actually. You just need to activate this option 'Allow activity stream commenting on blog and forum posts'. It looks like there's a problem with activity comment / blog comment syncing introduced in 2.0

#31 @DJPaul
3 years ago

It's because the bp_get_activity_content filter is also used in bp_get_activity_comment_content, which seems like a bad implementation of a good idea (to share common output filters between activity and activity comment objects). I don't feel like tackling that mess at the moment, so I've attached a patch which I think might be OK for now. What do you think?

#32 @imath
3 years ago

Just tested, it's good and i think you're right :)

#33 @djpaul
3 years ago

In 9648:

Activity: fix warning when commenting on post type activity when "Allow activity stream commenting on blog and forum posts" is enabled.

This was introduced in r9621 when the blog post excerpts were improved by integrating the media extractor.

See #6177

#34 @johnjamesjacoby
3 years ago

In 9650:

Activity: Clean up docblock for bp_activity_create_summary(). See #6177.

#35 @DJPaul
3 years ago

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

#36 @r-a-y
2 years ago

In 10834:

Core: Introduce bp_core_extract_media_from_content().

This is a convenience function to access the BP_Media_Extractor class,
which was made available in BuddyPress 2.3.0.

See #6177.

Note: See TracTickets for help on using tickets.