#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)
Change History (38)
#2
@
10 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.
#4
@
10 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
@
10 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)
#8
@
10 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
@
10 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.
10 years ago
#11
@
10 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.
#13
@
10 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.
10 years ago
#15
@
10 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
@
10 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:
↓ 21
@
10 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.
#18
@
10 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
@
10 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
@
10 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
@
10 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
@
10 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
@
10 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
@
10 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).
#27
follow-up:
↓ 28
@
10 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
@
10 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
@
10 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
@
10 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
@
10 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?
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.bp-activity-functions.php
is covered by #6159The "media-extractor-activity-stream" branch changes the Activity Stream to use the media extractor class to generate the richer Activity items.
bp-activity-filters.php
is a temporary solution; activity summaries that contained an audio shortcode produced too much HTML, and some part of this filter (and probablybalance_tags
) was removing the audio player HTML. I need to investigate further.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.