Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Updates to head component #56

Merged
merged 1 commit into from
Oct 9, 2018
Merged

Conversation

jameswburke
Copy link
Contributor

@jameswburke jameswburke commented Oct 9, 2018

Additional head tag updates

@jameswburke jameswburke merged commit 9b9bc62 into production Oct 9, 2018
@jameswburke jameswburke deleted the modify-head-component branch October 9, 2018 20:36
Copy link

@tomharrigan tomharrigan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple suggested changes. 🐹

return $meta_description;
}

// Modify global state.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following should accomplish the same without modifying global state and having to set the temp variable apply_filters( 'get_the_excerpt', get_post_field( 'post_excerpt', $post_id ) )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a long discussion with @mboynes about this and this was his recommended approach. He suggested avoiding the filter all together.

public function get_social_image_url( $post_id, $photon_args = [] ) {

// Get image url.
$image_id = absint( get_post_meta( $post_id, 'social_image_id', true ) );

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be good to return early if this isn't valid to avoid a wp_get_attachment_image_url call

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants