[buddypress-trac] [BuddyPress Trac] #6772: BuddyPress Embeds for activity, user profiles, groups

[buddypress-trac]

#6772: BuddyPress Embeds for activity, user profiles, groups
------------------------------------+------------------
 Reporter:  imath                   |       Owner:
     Type:  idea                    |      Status:  new
 Priority:  normal                  |   Milestone:  2.5
Component:  API                     |     Version:
 Severity:  normal                  |  Resolution:
 Keywords:  dev-feedback has-patch  |
------------------------------------+------------------

Comment (by imath):

 @r-a-y first, thanks a lot for `05.patch` it does fix the issue i was
 having with `04.patch`.
 I really like the class you added to abstract BP Embeds, great work!

 Thanks a lot for your suggestion about the filter to disable embeds in
 embeds and the `hide_media` query args to eventually display them. But i'm
 sorry, the fluidvids.js workaround is an *absolute no go* for me:
 - because the version added seems outdated, it's saying `v1.2.0` in the
 inline script added although it seems stable version is now
 [https://github.com/toddmotto/fluidvids/blob/master/dist/fluidvids.js
 2.4.1]
 - because, the copyright is outdated and we would need to include a
 license as it's MIT
 - because it only solves the issue for embedded youtube or vimeo videos,
 see the following cases for Dailymotion or WordPress.tv

 [[Image(https://cldup.com/rvKQSt_qxv.png)]]
 Dailymotion

 [[Image(https://cldup.com/g9mVBrbzKI.png)]]
 WordPress TV

 - because embed content can be videos, but also other kind of content, so
 even if we were adding players as it's explained
 [https://github.com/toddmotto/fluidvids#init here], it would only work for
 videos.

 So i'm feeling a lot better with the following kind of output, because i
 think it would be too complex to make sure embeds in embeds are working
 great for any kind of embeds.
 [[Image(https://cldup.com/tDxun6fUpG.png)]]
 Read more...

 I think we should apply most of the `bp_get_activity_content_body` filters
 to the excerpt to be sure it's well displayed (in the first screenshot,
 you can see that slashes are output without them).

 I think we should find something better for the css for embeds (see
 suggestions in the patch)

 In BP_oEmbed_Component->filter_embed_html `__( 'Embedded WordPress Post'
 )` is not passing the commit grunt task because of a missing text domain.
 Do you think there's another way to replace the title attribute of the
 iframe ?

 Then i'd like to come back on a few concerns `05.patch` didn't replied to.

 > I think maybe we should `bp_` prefix the embed_content hook inside your
 template as it's used by WordPress, or maybe use a specific template tag
 instead.. But maybe you did it on purpose to let any plugin adding content
 to post embeds to also put this content into the activity embeds ?

 The WP Action `embed_content` is there to add content to the output *once*
 the main content has been printed. What we're doing here is printing the
 main content. I think we should prefix this action in favor of
 `bp_embed_content` because i think people filtering `embed_content` will
 try to get informations about the post like `get_the_ID()` etc..

 So `.05.suggestions.patch` is containing some suggestions, you'll need to
 apply `05.patch` and
 [https://buddypress.trac.wordpress.org/attachment/ticket/6778/6778.02.patch
 6778.02.patch] first for it to work. The last point is not included in the
 patch

 > Also, i was thinking about the date at the bottom, what about putting it
 in the embed title like imath posted on December... ?
 I know Twitter is doing like you're suggesting so far: put the
 date/timestamp under the excerpt, but as we're building BuddyPress embeds,
 i think we should consider moving this part near what normally is the
 WordPress title & link to the content.

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/6772#comment:17>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac