[buddypress-trac] [BuddyPress Trac] #6772: BuddyPress Embeds for activity, user profiles, groups
buddypress-trac
noreply at wordpress.org
Wed May 18 22:28:15 UTC 2016
#6772: BuddyPress Embeds for activity, user profiles, groups
------------------------------------+------------------
Reporter: imath | Owner:
Type: idea | Status: new
Priority: normal | Milestone: 2.6
Component: API | Version:
Severity: normal | Resolution:
Keywords: dev-feedback has-patch |
------------------------------------+------------------
Comment (by imath):
@r-a-y thanks you so much for your continuous great work on this ticket.
About iframe in iframe: i confirm the security issues are fixed. So 🚦===
green light :)
Activity embeds are looking really great, i only have some last
suggestions.
1. i think we should try to avoid adding too much inline css or js in php
code. I see no reason for the embed stylesheet to be private, it seems too
bad and anyway we can always override it with !important, so i think we
can use `wp_enqueue_style()`, enjoy rtl, and include a filter if the
themes want to customize it. It will avoid super charging css. I think we
can use `wp_enqueue_script()` for fluidvids too. This way we are actually
using the function you created `bp_enqueue_embed_scripts()` (you were not
using it in 08.patch)
2. About fluidvids.js as activity embeds requires 4.5 i think we should
enjoy `wp_add_inline_script()` to set fluidvids options.
3. About the `assets/embeds/header.php`, now that you created
`assets/embeds/header-activity.php` it's not loaded anymore. But maybe
you're using it as a fallback, so i've left it but i removed the mention
name because it wasn't looking great.
4. About `assets/embeds/header-activity.php` i think the timestamp should
display even if mentions are disabled. So i've edited the template that
way.
5. About the activity loop fired twice (in `assets/embeds/header-
activity.php` & `assets/embeds/activity.php` ) i suggest to introduce a
new wrapper `bp_activity_embed_has_activity()`. This way if the
`$activities_template` is already set for the embed activity, the
`assets/embeds/activity.php` can directly use it without resetting the
loop.
.08.suggestions.patch requires .08.patch and contains the above
suggestions. (sorry i haven't include the docblocks but i wasn't sure
you'd be ok with theses suggestions, so i've been lazy!)
Finally, i now feel pretty confident about this great feature. Thanks
again for your perseverance.
--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/6772#comment:34>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac
More information about the buddypress-trac
mailing list