Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Microformats 2 Rendering #30

Open
dshanske opened this issue Jun 27, 2017 · 71 comments
Open

Microformats 2 Rendering #30

dshanske opened this issue Jun 27, 2017 · 71 comments

Comments

@dshanske
Copy link
Member

@snarfed, @pfefferle and I were discussing the problem of various plugin interoperability in pfefferle/SemPress#51

It occurred to me, if we are going to set this up, this repo is probably the better place to keep the discussion and possibly any functionality or such we want to support it.

Right now, Micropub takes over rendering if add_theme_support( 'microformats2' ) is not set. This plugin loads microformats2 compatibility functions(add h-entry, etc) if the same flag is set.

So, the add theme support flag should, in future, be used solely to discuss a theme's support for marking up standard WordPress elements, as opposed to properties stored solely in meta. We can be more granular in future if needed.

Rendering microformats2 properties from Post Meta probably requires a global variable(the same way that add_theme_support, register_post_type, etc are in reality global variables).

The issue that was being discussed was priority on any given feature. WordPress registration does this with the last one to register, as registration is done by hook. Same can apply here.

While I still think we can combine our rendering efforts, for now, I want to try to build this as a class here in Microformats 2, which could then be included in the other plugins. Thoughts?

@pfefferle
Copy link
Member

thanks for summing this up!

@snarfed
Copy link
Member

snarfed commented Jun 27, 2017

hey, great! happy to talk wherever. and expanding to also include wordpress element markup sounds fine too.

i do still think we need to support priority, though. the micropub plugin, for example, needs to be able to render all mf2 properties that it handles, but it also should defer to any other feature-specific plugin that handles an individual mf2 property, since those plugins will generally be better.

i think we also need this coordination to work regardless of which specific plugins are installed. SemPress users, for example, often won't have this uf2 plugin, but should still be able to use micropub and other feature-specific plugins together happily.

given all that, the global variable approach seems to have two flaws:

  • no priority support.
  • no obvious single place to actually create the global variable(s). we could make all plugins create it (them) if necessary, but that seems error prone at best.

i'm not excited about the feature approach described in pfefferle/SemPress#51 (comment) either. it requires all plugins that use it to check and use the filter return value correctly. still, it may be the least bad option we have so far...?

@dshanske
Copy link
Member Author

@snarfed That's why I suggested this repo as the 'canonical' place for any microformats2 in wordpress discussion. The lack of single place to discuss things.

This is really where plugin dependencies would help a great deal, but saying that doesn't help with this. But why can't SemPress users, Micropub users, and Post Kinds users be encouraged to install this plugin, especially if it doesn't activate the extra features if the theme says it already supports them?

I'm worried about not only creating confusion for us, but for new people(who already find it confusing). I don't think there is an easy answer, but what we have now isn't working exactly.

@pfefferle
Copy link
Member

My idea:

I agree with @snarfed, every plugin should handle it's properties, the best it can.

The main other authority for the rendering is the theme, so I would start defining the add_theme_support( 'microformats2-rendering', array( 'mf2_photo', 'mf2_name' ) ); discussed here: pfefferle/SemPress#51 (comment)

If a plugin like indie-post-kinds want to overwrite micropub functionality, it should be done individually be remove actions or filters or with changes done by filters and actions.

@dshanske
Copy link
Member Author

I might not want to. @snarfed's rendering could be nicer than mine.

Why 'mf2_photo' as opposed to 'photo' ?

@pfefferle
Copy link
Member

If the theme is the only authority it is even better.

The syntax of the theme support was only a first shot, I would highly recommend to discuss and change the feature flags!

@dshanske
Copy link
Member Author

The theme being the top authority makes sense...as long as the theme knows how to do something. If it doesn't, it needs to fall back on something.

@pfefferle
Copy link
Member

sure, and that can be handled with more granular theme support flags.

@dshanske
Copy link
Member Author

So, 'microformats2' as a theme_supports flag will have arguments for WordPress properties..For example, author, content, etc.

'microformats2_rendering' would have arguments for properties that don't directly map to WordPress properties? Or do we want to combine them into one single microformats2 flag with the differences set in properties?

We still, either way, end up with collision.

@pfefferle
Copy link
Member

exactly. i would use microformats2 (because of its adoption) for both

@dshanske
Copy link
Member Author

Then prefixing the mf2 properties with mf2_ makes sense, I suppose.

@pfefferle
Copy link
Member

ah, now I know why I proposed a new rendering flag... Let me think about that... perhaps we should create a section on the wiki, where we collect the different ideas and where we can define the flags. I hope @snarfed is ok with this too.

@dshanske
Copy link
Member Author

Why not have all possible plugins register support, and have the user specify their priority for applying them if both have support for that property? For example, both Post Kinds and Micropub register they support rendering checkins. The user specifies in configuration that they'd prefer rendering in the event of both?

@miklb
Copy link
Contributor

miklb commented Jun 27, 2017

-1 to putting the onus on the user to specify priority. They aren't going to want that responsibility, they just want an integrated experience. Just sounds like opening up a bad UX with a ton of options and configurations.

@dshanske
Copy link
Member Author

I'm suggesting 1 option...who has priority. Not on a per property basis.

@dshanske
Copy link
Member Author

We can call it, "Specify Default Rendering Plugin." or such. Rather than saying that each plugin sets a priority on a per property basis. It seems like that is too many low level settings.

@dshanske
Copy link
Member Author

https://indieweb.org/WordPress/Data - Is where we've put this stuff on the wiki before. I'll go garden a bit.

@miklb
Copy link
Contributor

miklb commented Jun 27, 2017

Playing devil's advocate, a user is going to ask why would I need to specify default rendering? We already hear routinely "why not just one plugin". Requiring a user to specify even once what plugin gets priority is just going to exacerbate that.

@dshanske
Copy link
Member Author

Then we're right back at, why not spin rendering out of all plugins into its own?

@miklb
Copy link
Contributor

miklb commented Jun 27, 2017

I don't begin to have an answer to the how, and as I stated in chat, it's a hard problem you are trying to solve with multiple plugins. But one worth tackling. I'm just trying to provide some perspective from user stand point based on feed back I've seen and read.

@pfefferle
Copy link
Member

pfefferle commented Jun 27, 2017

But that is what I meant... the main renderer is the plugin itself but it can be disabled by the theme... if a second plugin thinks it's a better renderer it should handle it individually using hooks and actions...

@pfefferle
Copy link
Member

do we have cases where a plugin uses data from an other plugin?

@dshanske
Copy link
Member Author

Yes, Post Kinds uses any data stored in mf2_ whether it stored it or not. Micropub also stores there. But as I documented, there are some issues.

@pfefferle
Copy link
Member

pfefferle commented Jun 27, 2017

I mentioned that in a comment above and you corrected me:

If a plugin like indie-post-kinds want to overwrite micropub functionality, it should be done individually by remove actions or filters or with changes done by filters and actions.

Perhaps a misunderstanding...but as I said, this can be done individually, like we do with webmentions and semantic linkbacks https://github.com/pfefferle/wordpress-semantic-linkbacks/blob/master/semantic-linkbacks.php#L52 or am I wrong?

@dshanske
Copy link
Member Author

It can. I just want to brainstorm all options before we adopt. It is harder to change later.

@pfefferle
Copy link
Member

ok, got you, but we shouldn't over engineer the feature on the other hand! plugins working really good together at the moment and we only have a problem to identify if a theme supports render features or not!

@pfefferle
Copy link
Member

... and we should fix that in time... at least before we start investing time in new wp-uf2 features, because we currently only know if a plugin supports microformats2 or not and this might cause issues like with sempress and micropub.

@snarfed
Copy link
Member

snarfed commented Jun 27, 2017

hey @kraftbj, you've been interested in indieweb recently, any chance you could give us some expert guidance here? no good deed goes unpunished, and all that. :P

we're still discussing the exact goals, but broadly, we're trying to figure out how multiple plugins/themes should coordinate and decide, at post render time, which one renders a specific piece of mf2. here are some constraints:

  • for each piece of mf2 in the post, we want at most one thing to render it, either the theme or a single plugin. (if nothing is available to render it, that's fine.)
  • we don't want to assume that any single specific plugin or theme is installed. (ie, we don't have a single obvious master coordinator.)
  • we want plugins/themes to declare their own priority, and have the highest priority win and render.

options we've considered:

  • filters. straw man design: if the value passed to the filter is null, render and return the rendered HTML. otherwise return it unchanged. nice, built in, no need for a master coordinator, supports priority...but requires all plugin/theme authors to implement that logic (correctly).
  • registration into global config var, a la post types. similarly requires every contributing plugin to implement the registration code and global var.
  • ...?

i'm sure other people have figured this out before, but i haven't had much luck googling for it (e.g. wordpress multiple plugins coordination :/). any ideas?

thanks in advance!

@kraftbj
Copy link
Contributor

kraftbj commented Jun 27, 2017

That's a dilly of a pickle. I would venture that this is something that's ripe for a filter--it already has the built-in priority system.

With both issues, it requires buy-in from other plugin/theme authors to use a common format/scheme. I think the filter is more akin to WordPress thinking. Let me read through this issue, the core ticket that was attempted for mf2 support, etc with more thought some.

@snarfed
Copy link
Member

snarfed commented Jun 27, 2017

another thought: is there any way to get the full list of callbacks registered for a given filter, along with their priority? if so, we could use that and just call the first one. kind of goes against the philosophy, i know, but still.

i didn't find anything like that in the docs, so i'm guessing not, but figured it was worth checking.

@pfefferle
Copy link
Member

pfefferle commented Jun 29, 2017

My intention was to kill all plugin processing if the theme already supports this type of rendering.

add_theme_support( 'microformats2' , array( 'author', 'mf2_photo' ) ); should tell all plugins that the theme supports author-markup and that it can render the mf2_photo post-metas for example... so plugins should skip rendering for these two features.

I don't like the add_filter('mf2-no-plugin-rendering', '__return_true' ); because this results in the same problem we have now... mf2 rendering can only be "on" or "of", but we need a more granular system.

What a theme could provide is something like add_filter( 'mf2-features', array( 'author', 'mf2_photo' ) );, so the plugin can check by adding the apply_filters( 'mf2-features' ); but this feels a bit like the wrong way round...

@miklb
Copy link
Contributor

miklb commented Jun 29, 2017

but this feels a bit like the wrong way round...

I can't speak to how it fits core philosophy, but from someone who would be a consumer of that filter, it makes sense.

@pfefferle
Copy link
Member

pfefferle commented Jun 29, 2017

It makes sense for you that a bunch of plugins have to init the exact same filter and one single theme is setting it?

I think this might cause some serious race condition problems, because you do not have a priority on the apply_filters, only on the add_filter.

If you switch it around, so that the theme applies the filter, the theme has all the knowledge, that the plugins need.

@kraftbj
Copy link
Contributor

kraftbj commented Jun 29, 2017

I don't like the add_filter('mf2-no-plugin-rendering', '__return_true' ); because this results in the same problem we have now... mf2 rendering can only be "on" or "of", but we need a more granular system.

Trying to give theme authors a master kill switch. It was an afterthought to avoid a theme author needing to declare everything if they wanted total control...trying to meet you in the middle on our philosophical divide on if the theme should be the end-all of markup :)

add_theme_support( 'microformats2' , array( 'author', 'mf2_photo' ) ); should tell all plugins that the theme supports author-markup and that it can render the mf2_photo post-metas for example... so plugins should skip rendering for these two features.

Right, but if there's another plugin that extends upon wordpress-uf2, it can't redeclare add_theme_support afaik if it needs to.

What a theme could provide is something like add_filter( 'mf2-features', array( 'author', 'mf2_photo' ) );, so the plugin can check by adding the apply_filters( 'mf2-features' ); but this feels a bit like the wrong way round...

I think this is basically my idea, but the plugin would have a helper function to deal with it. For example, a theme author could do the full declarative array (like your example, but would need a callable since the array can't be declared directly there), but could easily just do add_filter('mf2-features-author', '__return_true'); if it wanted to.

I think this might cause some serious race condition problems!

It all depends on where it is hooked. All of the add_filter happen as the plugins/themes are loaded and helper function (with the apply_filters) fire on runtime within the_content, etc action... it should work.

@miklb
Copy link
Contributor

miklb commented Jun 29, 2017

It makes sense for you that a bunch of plugins have to init the exact same filter and one single theme is setting it?

Well, when you put it that way, no 😄

I'd guess there would be priority on the filter, which would help, I think @snarfed said he'd want micropub to only output if no other plugin was using the data for instance.

I was looking as a consumer of the code add_filter( 'mf2-features', array( 'author', 'mf2_photo' ) ); in that it makes sense when I read that filter.

@pfefferle
Copy link
Member

I am not sure we are talking about the same things (sorry I am German) ;)

With the add_theme_support I, as a theme developer, want to have a way to disable some renderings of plugins! The add_theme_support should not be extended or changed by any plugin and it should not be a global registry where plugins should hook in, to register themselves for a specific use case. If two or more plugins are fighting for the same output, this should be their way to find a solution (for example with remove filters and/or priorities) and I am not sure if there should/can be a generic way, because the data for these specific renderings might differ so much between the different use cases.

@pfefferle
Copy link
Member

Ok, perhaps it makes sense to try to have a generic way also for plugins, put I am not sure if we should mix both.

@pfefferle
Copy link
Member

@kraftbj who should provide the apply_filter wrapper? And don't forget that the content is not the only place where data is changed.

@kraftbj
Copy link
Contributor

kraftbj commented Jun 30, 2017

I apologize for the delay. Headed out of town for a vacation next week so trying to get everything at work packaged up. I meant to draw a diagram of how this all fits together. If I'm not making sense after this comment, I'll do that for when I'm back :)

The indieweb/wordpress-uf2 plugin (wherever, the "plugin"):

  1. Has the has_mf2_support function I psuedo-coded above wrapped in a function_exists call.
  2. The code that outputs microformats checks has_mf2_support( 'photo') before outputting.
  3. In short, "apply_filters" would happen in the plugin, but all within that helper function to 1. help keep logic in once place and allow for both kinds of filters I outlined (or all three if you include the kill switch, but I'm fine with ditching that).

Any theme that wants to declare support:

  1. Has a add_filter call in functions.php ( as demo'd in Microformats 2 Rendering #30 (comment) )

As far as loading:

  1. The plugin wraps the call in an function_exists just to be safe in case something else is declaring it too... a forked version of the plugin or whatnot.
  2. The theme is only using add_filter, so it doesn't really need to care if the plugin is installed or not. Worst case, it's a filter added that's never checked. No biggie.
  3. As long as the rendering is happening after the theme is loaded (anytime after after_theme_setup), it should be fine. The only problem would be if a plugin is declaring support and does it in some odd hook (vs just when the file loads). Any rendering would be happening later, after the query is parsed, etc,.
  4. Any confusion of priority would be resolved using WP Hook's filter priority system, if such a situation would even come up.

The theme could disable the plugin by passing the features you support. If there is some specific setups where a feature needs to be disabled in specific contexts, a plugin/theme can add a filter later, e.g.

add_action('pre_get_posts', 'bk_selective_disable');
function bk_select_disable(){
if ( is_page() ){
add_filter( 'mf2-features', 'bk_mf2_features' );
}
}
function bk_mf2_features( $features ) {
$features[] = 'photo';
$features[] = 'something';
return $features;
}

In other words, it can wait until the query is parsed and the conditional functions are available, then declare support if there is only support in certain contexts. Hooking on pre_get_posts or wp or something between plugins/theme loading and wp_head should allow for things to be modified before output.

@pfefferle
Copy link
Member

I would prefer a solution that can be implemented without relying on a third party plugin! If someone have not installed the wp-uf2 plugin, the whole functionality will not work. And until we have no IndieWeb plugin bundle, we could not control this case.

@miklb
Copy link
Contributor

miklb commented Jul 3, 2017

As a follow up, per a conversation in #indieweb-wordpress with pfefferle:

"so I see uf2 not as a manager, but as another participant, like semantic-linkbacks, micropub and indie post kinds."

I hadn't caught that in the thread, and think it helps clarify the discussion.

I thought the idea was to move to the mf2 plugin playing a larger role in the ecosystem, and with the ability to have themes "turn off" certain features, there would be encouragement to run it along side a microformats2 theme.

@kraftbj
Copy link
Contributor

kraftbj commented Jul 4, 2017

I don't understand the comment about the third-party plugin. This solution would mean the theme would 100% work stand alone OR alongside any plugin that uses this framework. I mean, sure, use theme-supports instead. Same idea just most limited with less flexibility.

If any plugin wanted to go along with this idea, it can include the same function wrapped in a function_exists call to avoid double-declaration.

Another alternative is just straight up say a theme works with a third-party plugin or say it doesn't period.

At the end of the day, I don't understand. We want a functionality that works with third-party plugins but using a third-party plugin is a non-starter?

@kraftbj
Copy link
Contributor

kraftbj commented Jul 4, 2017

When I'm back from vacation, I'll put together a basic suite showing the interaction in code.

@snarfed
Copy link
Member

snarfed commented Jul 5, 2017

thanks so much for talking and working through this, everyone! sounds like you may actually all be agreeing on the same general idea, and disagreeing at most on terminology and semantics. good sign!

you're all closer to wordpress than i am, so i trust you and i'll happily follow (in the micropub plugin) whatever you all settle on. especially hoping it satisfies these three goals of mine, but even if not, i'm flexible.

  • for each piece of mf2 in the post, we want at most one thing to render it, either the theme or a single plugin. (if nothing is available to render it, that's fine.)
  • we don't want to assume that any one specific plugin or theme is installed. (ie, we don't have a single obvious master coordinator.)
  • we want plugins/themes to declare their own priority, and have the highest priority win and render.

thanks again all!

@snarfed
Copy link
Member

snarfed commented Jul 23, 2017

checking back in here. is there a conclusion on the common design we all want to implement in our plugins and themes? i'm ready to implement it in the micropub plugin!

@snarfed
Copy link
Member

snarfed commented Aug 5, 2017

friendly ping!

@pfefferle
Copy link
Member

Sorry, but I off for vacation for the next two weeks...

@snarfed
Copy link
Member

snarfed commented Aug 6, 2017

no worries at all. enjoy!

@snarfed
Copy link
Member

snarfed commented Aug 24, 2017

friendly nudge! @dshanske @kraftbj @miklb @pfefferle do we have a conclusion here on a design we all agree on yet?

@pfefferle
Copy link
Member

not that I know...

@dshanske
Copy link
Member Author

It just occurred to me. If we want to surface this to a user, that means shouldn't we be using the options table? It is a user option.

So, in the event an option isn't set, there are now 2 ways to set the default.

  1. The old way: get_option - https://developer.wordpress.org/reference/functions/get_option/
  2. The new way: register_setting - https://developer.wordpress.org/reference/functions/register_setting/

What about:

  • If microformats2 theme support is set and option is not set, default to theme handling all properties
  • If microformats2 theme support is not set and option is not set, plugin to set option on plugin activation
  • If neither microformats2 theme support is set and option is not set, this means someone changed themes, from mf2 to non mf2 therefore surface an admin notice to get them to set the option.
  • If option is set and microformats2 theme support is set, on plugin activation, surface admin notice to user advising that another plugin has registered microformats2 rendering support and to go to settings if they want this plugin to handle it.
  • On deactivation of plugin, retrieve option and remove any reference to that plugin handling that property.

No matter what, every plugin and theme would have to implement code the logic regarding it, although we could have a repo with the code in question and include it as a dependency. If we write it correctly, then it shouldn't need to be changed.

@miklb
Copy link
Contributor

miklb commented Dec 23, 2017

Just so I understand:

I write a theme and I set an option saying "This theme handles all mf2 output."

Someone installs and activates this plugin (wordpress uf2). They get an admin notice telling theme "hey your theme is already configured to output mf2 data, you do not need to activate this plugin."

User then activates hot new IndieWeb plugin. Hot new plugin sees that the theme is handling output and doesn't display anything on the front end?

That's then assuming the theme author is familiar with hot new plugin and the user will still get the desired result?

Or does that just signal to the plugin, "hey this theme is mf2 ready, fire output at will."

Or something entirely different?

@dshanske
Copy link
Member Author

All really good points. So...

  1. The option is that the theme supports microformats2. One would assume that themes that care would, over time, set themselves up with specific properties they support in the same option. But initially, shouldn't the assumption be, without any other data, they know what to do?
  2. User activates new plugin, and I wouldn't say the new plugin couldn't, on activation, surface an input to the user to setup configuration options if it wants?

No matter what, new things will break old things. This has to be a change we'll all support some form of as soon as possible, no matter which one we go with. So, with that in mind, anyone who doesn't support this new option, however we implement it, what should we do? The safest thing is to back off if they didn't declare more.

@miklb
Copy link
Contributor

miklb commented Dec 24, 2017

I guess from a theme maker standpoint, as long the plugins have a way for the theme to override a plugin output, default to outputting by the plugin would make sense. So if Hot New plugin displays something my theme doesn't, in a future theme update I could override the plugin and display it differently if necessary.

@snarfed
Copy link
Member

snarfed commented Jan 8, 2018

woo! we all met today and agreed to try out this proposal: #30 (comment) .

we also discussed the broader plan for indieweb wordpress plugins and themes in general. we settled on doing more in the official indieweb plugin, ideally focusing on a wizard style UX. @dshanske and @miklb let me twist their arms into eventually drafting a design doc for how that should look.

thanks everyone!

pfefferle added a commit that referenced this issue Feb 18, 2018
so that the plugin can handle the theme-support. see #30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants