Image2: Linked inline image's alignment class is not transferred to widget wrapper

[Piotrek Koszuliński]

Source:

<p>
	<a href="http://en.wikipedia.org/wiki/File:Comet_67P_on_19_September_2014_NavCam_mosaic.jpg"><img src="img/comet-67p.jpg" alt="Comet 67P" width="1131" height="900" class="image30 align-right" /></a>
</p>

Settings:

	image2_alignClasses: [ 'align-left', 'align-center', 'align-right' ],
	image2_disableResizer: true,

	stylesSet: [
		{ name: 'Image 30%', type: 'widget', widget: 'image', attributes: { 'class': 'image30' } }
	]

[Piotrek Koszuliński]

Compare this with a not linked image case. With link the class is left on the image, without link it's properly transferred to the wrapper.

[Artur Delura]

I found curious relationship: If we add some text content (even one space) between closing img and a tag then everything is fine. There are two diffrences. Anchor is moved out of the widget wrapper and wrapper itself contains desirable class.

The question is whether anchor tag should remain in widget wrapper or it might be kicked out from it? Think it's good to have link out of the image wrapper. I don't see a reson to keep it in.

[Piotrek Koszuliński]

That behaviour is not a surprise. Image2 accepts links only as it (itself) would create them - without any spaces. That's totally correct and you can ignore links with spaces in this ticket. The question that can be asked is - why do we keep links inside widgets. I remember that there were important reasons, but @anowodzinski could definitely write more.

[Artur Delura]

Moving back to issue:

When widget is created there is set a data object on it. Data object has propety align which is set to none. Aling setup is ​here. It look for specified classes in this.element which in our case is an anchor, which obviously doesnt' have such classes, so finally align is set to none.

Here comes another question whether property element has proper value - which is an anchor. Shouldn't it be an image? I think the answer is: This is okay.

I assume that this is okay. In code listed above, we can look through not only this.element but it's child as well. Which in our case is an image which have our class which we are looking for.

[Artur Delura]

Changes and tests in ​branch:t/13197.

[Piotrek Koszuliński]

I was close to giving R+, but I don't like one thing - blindly checking if any element of the widget has an alignment class.

if ( this.element.hasClass( alignClasses[ 0 ] ) || image.hasClass( alignClasses[ 0 ] ) )
    data.align = 'left';
else if ( this.element.hasClass( alignClasses[ 2 ] ) || image.hasClass( alignClasses[ 2 ] ) )
    data.align = 'right';

I would rather see there a check - if it's a captioned image, then look for class on this.element. If it's not a captioned image, always look on image.

[Artur Delura]

Changes in ​branch:t/13197.

[Piotrek Koszuliński]

I removed the 2nd case from the manual test because it wasn't necessary. If anything, it should be tested by automated tests. Besides that - the fact that alignment class work only on image depends only on configuration. If you changed ACF and contents.css everything could be fine.

Fixed on major with ​git:ce8ccc2.