img_caption_shortcode_width should support percentage widths

[danielbachhuber]

Currently, it assumes the returned value to be pixels.

[hiwhatsup]

Maybe something like this might work? Thoughts?

	if ( strpos ( $atts['width'], '%' ) !== false ) {
		$caption_width_um = "%";
	} else {
		$caption_width_um = "px";
	}
	$atts['width'] = (int) $atts['width'];

	if ( current_theme_supports( 'html5', 'caption' ) ) {
		return '<figure ' . $atts['id'] . 'style="width: ' . (int) $atts['width'] . $caption_width_um .  ';" class="' . esc_attr( $class ) . '">'
		. do_shortcode( $content ) . '<figcaption class="wp-caption-text">' . $atts['caption'] . '</figcaption></figure>';
	}

Since by default, the caption is 10 pixels greater than the width of the image. We'll need to add those 10px using CSS3 function calc(). I'm not how well this will work on older browsers. Another option is to add a margin or padding.

	if ( $caption_width ) {
		if ($caption_width_um == "%") {
			$style = 'style="width: calc('. (int) $caption_width .'% + 10px); width: -moz-calc('. (int) $caption_width .'% + 10px); -webkit-calc('. (int) $caption_width .'% + 10px);  -o-calc('. (int) $caption_width .'% + 10px); " ';
			}
		else {
			$style = 'style="width: ' . (int) $caption_width . 'px" ';
			}
		

	return '<div ' . $atts['id'] . $style . 'class="' . esc_attr( $class ) . '">'
	. do_shortcode( $content ) . '<p class="wp-caption-text">' . $atts['caption'] . '</p></div>';
	}

This is my first time here after attending @wcsf14. I wanted to ask before committing a diff and get some feedback for veteran committers and contributors.

[williampatton]

I'm not exactly a veteran contributor or anything but I'm not certain that using a css3 function is the best idea for this as there's still a fair amount of people stuck using browsers that don't support css3 at all. Worried it wouldn't degrade gracefully for those users.

I prefer your suggestion of using margin/padding instead as the solution to add the extra space.

[Yahire Furniture]

Would It not be easier to just keep the style of the caption to within the css?

[williampatton]

Not possible to keep it purely within the css (I assume you mean a stylesheet) as it needs to output a user specified value.

[polevaultweb]

This patch adds a right margin of 10px when a percentage width is returned from the filter. This is my first patch/contribution/trac usage so go easy :)

[kevdotbadger]

Isn't the margins added automatically when the alignleft and alignright classes are added? @hiwhatsup's solution seems to work nicely, however tinyMCE strips away the % from the shortcode, so that needs patching as well. the current regex is /width=['"]([0-9]*)['"] ?/ which matches just numbers, we need to amend that so that it matches numbers as well as %.

I think a new attr might be the best way, a 'caption_width_unit' one which defaults to px for backwards compatibility, and allows either blank (as default), 'px' or '%'. This way we could also add em or whatever later.

Thoughts?

[azaozz]

How would the percentage width be used? Limiting with max-width (in combination with @media) seems to cover nearly all user cases?

Also, percentage width on a big screen might stretch it over the image size.

[danielbachhuber]

IIRC, we were doing something with 50% width (and float left / right). We used media queries to set lower and upper bounds.

[azaozz]

Replying to danielbachhuber:

There is no good way to use default percentage width without also setting min and max width. It will also force scaling of the image, possibly up-scaling in some cases.

Thinking this is best left for the theme to implement. I know it may need an !important rule, but probably warranted here. Suggesting wontfix.

[williampatton]

Replying to azaozz:

I don't think that anyone is suggesting a default percentage width - more that they would like the option of setting a percentage if the usage called for it. The default should still be pixel based as defined by the theme image sizes.

Also I agree that percentage widths don't have many good usage cases unless the theme specifies a min-width/max-width but don't think we should totally disregard this idea because of that. That being said your the one here with most experience of developing WP so you're probably the one in the best position to make that decision :p

[azaozz]

Replying to williampatton:

I don't think that anyone is suggesting a default percentage width...

I meant supporting a percentage width for image captions as an option in TinyMCE, sorry for not making it clearer. It's not acceptable to have it work one way in the editor and another way on the front-end. Currently this is possible to do both in the editor and on the front-end with width: 50% !important, etc. and IMHO this covers the user cases well.

[williampatton]

Replying to azaozz:

I meant supporting a percentage width for image captions as an option in TinyMCE, sorry for not making it clearer. It's not acceptable to have it work one way in the editor and another way on the front-end. Currently this is possible to do both in the editor and on the front-end with width: 50% !important, etc. and IMHO this covers the user cases well.

Ahh OK I get you. To be honest I hadn't realized there would be a discrepancy between editor and front-end. Being someone who's more comfortable using the text editor than the visual one I often overlook issues like that.

There are a handful of cases where allowing a percentage to be used would be handy but I'm starting to see your point and edging more towards your wontfix suggestion. Themes that set something like width:50% !important definitely would cover most use cases - and since [caption] can accept a custom class name I think that offers an acceptable way to set anything else that might be required.

I second the notion of 'wontfix' for this ticket but can't help thinking that there's got to be something that could be done for anyone that does need to be specify percentages for caption sizes... unfortunately I'm not really sure what that 'something that could be done' might be :(

[polevaultweb]

Replying to azaozz:

Thinking this is best left for the theme to implement. I know it may need an !important rule, but probably warranted here. Suggesting wontfix.

Ok so I have been assigned this ticket, but are we thinking its a wontfix? Happy to defer to your judgement on this @azaozz

[wonderboymusic]

as suggested by the owner