Closed imath closed 8 years ago
things are going to get very messy and hard to maintain with odd style files dotted around, is this what we've done in core? or is this just temporary until styles approaches are decided.
This is what has been done in core.
ok sorry; just checking core and commit notes - so this embed and related css is for thirdparty sites? not loaded by the WP site - I have to look further because I'm not really getting this at the moment. Does feel as though templates are getting a tad too fiddly nowadays though in BP :(
Hmm, this issue makes me think that maybe we should adopt what WordPress does and have core theme-compat templates.
Is that a crazy idea?
About the embed CSS files, maybe we can move them from /bp-legacy/assets/embeds/
to /bp-legacy/css
to suit how other CSS files are located in bp-legacy?
hnla - Embed CSS is rendered inline to match what WordPress does in their embed templates.
@r-a-y
adopt what WordPress does and have core theme-compat templates.
tempted to agree - just trying to recall what or how WP allows for overload, comments templates work this way don't they?
I think style file moved to /bp-legacy/css
might be better - to what degree are these styles for required formatting as opposed to styles to make something look pretty for a theme? If the former then locate the styles and templates in core as long as template can be overwritten, styles do not need to be available to end users if they want to add flourished in design sense then add to a custom style file in your theme.
Embed CSS is rendered inline you mean
<style type="">
inline being as a style attr on an element
Style blocks in the doc body are awful shouldn't be allowed but hey ho :)
@r-a-y i kind of disagree with (at least for 2.6) :
adopt what WordPress does and have core theme-combat templates
My point on the 7115 ticket was to post on bpdevel to warn people using specific themes with their own template pack might not have the activity embed feature available if these themes hadn't updated their templates.
I'm kind of disappointed right now... It's been six months since @r-a-y posted the first patch on 6772 and it was already using the inline css stuff: see this ticket https://buddypress.trac.wordpress.org/ticket/6772
So i think this kind of feedback should have happened earlier and on the 6772 ticket. We shouldn't talk about this here.
This commit is there to make sure BP Nouveau is rendering the activity embeds. We'll adapt to what will happen in core, but it's now working!
If the way it's done in core is not Ok with you @hnla i'd suggest you open a ticket or comment on this ticket: https://buddypress.trac.wordpress.org/ticket/7116
Ok sorry folks, haven't been able to follow all activity on BP so a bit lost with this one at the moment, I'll look at and make comments if necessary on 7116
thanks @hnla ;)
The reason is, we need to add the
embeds
subfolder and all its content to ourbp-nouveau/buddypress/assets
folder so that the embedded activity is displayed.