This repository was archived by the owner on Apr 2, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 6
Proof of concept – using wp_get_attachment_image #71
Open
davisshaver
wants to merge
12
commits into
wp-shortcake:master
Choose a base branch
from
davisshaver:66-use-wordpress-function-for-responsive
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
eab03d7
Proof of concept – using wp_get_attachment_image
760bd4d
PHPCS issues
d098e40
Add basic test for srcset
d23777b
Updates based on code review
95c6a26
Add back return if no src or attachment
2b39fae
Try to remove twenty sixteen filter
2d75a46
Try forcing Twenty Fifteen
77efa36
Don't use deprecated `get_current_theme()`
58e0219
Use array_diff_key instead of unset
950cacd
Add trailing comma to $attrs_for_removal
ac6be1c
Try setting theme inline test just for laughs
b906886
Remove hack which obviously didn't work
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -176,37 +176,42 @@ public static function callback( $attr, $_null, $shortcode_tag ) { | |
| * @param array Shortcode attributes, decoded and merged with defaults. | ||
| */ | ||
| $attr = apply_filters( 'img_shortcode_attrs', $attr ); | ||
|
|
||
| $image_html = '<img '; | ||
|
|
||
| $image_classes = explode( ' ', $attr['classes'] ); | ||
| $image_classes[] = 'size-' . $attr['size']; | ||
| $image_classes[] = $attr['align']; | ||
| $image_classes_string = trim( implode( ' ', $image_classes ) ); | ||
| unset( $attr['classes'] ); | ||
|
|
||
| if ( isset( $attr['attachment'] ) ) { | ||
| $attr_for_responsive = $attr; | ||
| unset( $attr_for_responsive['caption'] ); | ||
| unset( $attr_for_responsive['attachment'] ); | ||
| unset( $attr_for_responsive['linkto'] ); | ||
| unset( $attr_for_responsive['url'] ); | ||
| unset( $attr_for_responsive['size'] ); | ||
| unset( $attr_for_responsive['align'] ); | ||
| unset( $attr_for_responsive['src'] ); | ||
|
|
||
| $attr_for_responsive['class'] = $image_classes_string; | ||
| $image_html = wp_get_attachment_image( $attr['attachment'], $attr['size'], false, $attr_for_responsive ); | ||
| } else if ( ! empty( $attr['src'] ) ) { | ||
| $image_html = '<img '; | ||
|
|
||
| $image_attr = array( | ||
| 'alt' => $attr['alt'], | ||
| 'class' => trim( implode( ' ', $image_classes ) ), | ||
| ); | ||
| $image_attr = array( | ||
| 'alt' => $attr['alt'], | ||
| 'class' => $image_classes_string, | ||
| ); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the |
||
|
|
||
| if ( isset( $attr['attachment'] ) && | ||
| $attachment = wp_get_attachment_image_src( (int) $attr['attachment'], $attr['size'] ) ) { | ||
| $image_attr['src'] = esc_url( $attachment[0] ); | ||
| $image_attr['width'] = intval( $attachment[1] ); | ||
| $image_attr['height'] = intval( $attachment[2] ); | ||
| } else if ( ! empty( $attr['src'] ) ) { | ||
| $image_attr['src'] = esc_url( $attr['src'] ); | ||
| } else { | ||
| return; // An image without a src isn't much of an image | ||
| } | ||
|
|
||
| foreach ( $image_attr as $attr_name => $attr_value ) { | ||
| if ( ! empty( $attr_value ) ) { | ||
| $image_html .= sanitize_key( $attr_name ) . '="' . esc_attr( $attr_value ) . '" '; | ||
| foreach ( $image_attr as $attr_name => $attr_value ) { | ||
| if ( ! empty( $attr_value ) ) { | ||
| $image_html .= sanitize_key( $attr_name ) . '="' . esc_attr( $attr_value ) . '" '; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| $image_html .= '/>'; | ||
|
|
||
| $image_html .= '/>'; | ||
| } | ||
| /** | ||
| * Filter the output of the <img> tag before wrapping it in link or caption | ||
| * | ||
|
|
@@ -233,6 +238,11 @@ public static function callback( $attr, $_null, $shortcode_tag ) { | |
| // If a caption is specified, wrap the image in the appropriat caption markup. | ||
| if ( ! empty( $attr['caption'] ) ) { | ||
|
|
||
| // We need to generate width for WP captions | ||
| if ( isset( $attr['attachment'] ) && | ||
| $attachment = wp_get_attachment_image_src( (int) $attr['attachment'], $attr['size'] ) ) { | ||
| $image_attr['width'] = intval( $attachment[1] ); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd move this inside the |
||
| // The WP caption element requires a width defined | ||
| if ( empty( $attr['width'] ) ) { | ||
| $attr['width'] = $image_attr['width']; | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I suspect it'd be easier to
array_intersect_keywith a whitelist of allowed attributes than try to unset any potential attributes that wouldn't work here.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.
@goldenapples Agreed, but we would nullify any new attributes added in
img_shortcode_attrs. Kosher?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.
Ah, yeah. true. I can't think offhand what custom attributes you'd want to enable on an image, but in case there are any, we should allow them to pass through. At the very least, you could clean up this logic using
array_diff_key.