-
Notifications
You must be signed in to change notification settings - Fork 219
Add is_transforming_instant_article()
conditional
#568
Add is_transforming_instant_article()
conditional
#568
Conversation
Adds a new function in the global namespace to check if the current view being processed is for an instant article. Handles both the RSS feed and content which is being processed for the Instant_Articles_Publisher class. This conditional can be checked in any filters on 'the_content', 'the_title' or elsewhere that need to treat Instant Articles content separately from web content.
c76390a
to
8e54394
Compare
Rather than tracking down every place that might call Instant_Articles_Post::to_instant_article and wrapping it call to set the is_instant_article conditional, it's much simpler to just hook those settings on the existing actions `instant_articles_before_transform_post` and `instant_articles_after_transform_post`.
6a3dce7
to
f81998e
Compare
I know that this logic could easily be done in a theme or another plugin that interfaces with this, now that the actions before and after transforming post are available, but since this has been a common need of mine, it would be nice to have something like this available directly in this plugin itself. This would also make issues like #252 and #457 a lot simpler to do... |
is_instant_article
conditionalis_instant_article()
conditional
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love the simplicity of this solution. It'll be very useful indeed! I just think the method name needs to tell you it'll be true during the transformation phase more explicitly.
* @param bool Set the status | ||
* @return bool | ||
*/ | ||
function is_instant_article( $set_status = null ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about is_transforming_instant_article
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good sugeestion. I've updated the method name in aa33ecf.
@goldenapples in f81998e you attach |
Rename `is_instant_article` to `is_transforming_instant_article`, to better indicate that it will return true when a tranformation is being processed.
Since these hooks can be modified in plugins, it's safer to set the status of is_transforming_instant_article() directly in the `Instant_Articles_Post::to_instant_article` method.
@rinatkhaziev Good call. I was trying to port over the ad hoc solution I was using in a theme to the plugin. But if this method is going into the plugin directly, it does make much more sense to set the status directly rather than plugging in to those hooks. Does ca03891 look like what you had in mind? |
is_instant_article()
conditionalis_transforming_instant_article()
conditional
@diegoquinteiro This looks good to me, should we merge it into master and then I'm going to fix my PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rinatkhaziev done! =) |
Adds a new function in the global namespace to check if the current view
being processed is for an instant article. Handles both the RSS feed and
content which is being processed for the Instant_Articles_Publisher
class. This conditional can be checked in any filters on 'the_content',
'the_title' or elsewhere that need to treat Instant Articles content
separately from web content.
See #426