Original size and Alignment Options (including centering) patch [#89076]

Hi, I sort of rolled together http://drupal.org/node/59209 and http://drupal.org/node/70099 into a single patch. Besides adding the "none" alignment I also add a "center" alignment. I think it's not quite perfect but it's a good first stab. Besides the attached patch you will need to add the following rule to wherever you keep your style sheet:

span.center {
  margin: 0 auto;
  display: block;
}

Span's not a block level element unless you make it one, so it can't have a specified size, so you can't center it with automatic margins. Hence, we make it a block, and give it automatic left and right margins. This is not a floating block so content after it does not have to clear it.

I rolled these patches together because centering alignment involves working with the image size.

I decided to move the addition of the original size inside the check that the program uses to determine if you're allowed to use arbitrary sizes. This should ideally be a separate option but I haven't gotten there yet (and may not, since it doesn't really apply here.)

Comment	File	Size	Author
#10	img_assist-alignment-and-origsize_3.patch	4.16 KB	hyperlogos
#6	img_assist-alignment-and-origsize_2.patch	4.06 KB	hyperlogos
#4	img_assist-alignment-and-origsize_1.patch	4.57 KB	hyperlogos
#3	img_assist-alignment-and-origsize_0.patch	4.6 KB	hyperlogos
#1	img_assist-alignment-and-origsize-2.patch	3.21 KB	hyperlogos
	img_assist-alignment-and-origsize.patch	2.74 KB	hyperlogos

Comments

Comment #1

hyperlogos commented 13 October 2006 at 17:00

Status	File	Size
new	img_assist-alignment-and-origsize-2.patch	3.21 KB

I changed my mind and added a permission called 'use original size', and moved the code to insert the original size option so that it now requires this permission. (I think as it's written you might have to have "Access advanced options" as well, not sure. Not sure if this is desirable or not, either.) This is the only change between the patches, so whichever behavior the community would like is available and fine with me :)

Comment #2

hyperlogos commented 7 November 2006 at 18:52

Patch updated for CVS version (cvs, 23/10/2006 - 22:46)... somewhere along the line apparently something changed because the original size option used to work, and now it doesn't. Now, it works again. I'm now using only API calls to get the information; these calls use the installed image toolkit, whee.

This patch rolls together the original size and alignment patches (both "none" and "center") into a single patch.

Comment #3

hyperlogos commented 7 November 2006 at 18:52

Status	File	Size
new	img_assist-alignment-and-origsize_0.patch	4.6 KB

This patch rolls together the original size and alignment patches (both "none" and "center") into a single patch.

Comment #4

hyperlogos commented 7 November 2006 at 19:13

Version:

4.7.x-1.x-dev

» 6.x-3.x-dev

Status	File	Size
new	img_assist-alignment-and-origsize_1.patch	4.57 KB

Changing version to cvs, and I eliminated a needless variable so I'm resubmitting.

Comment #5

RobRoy commented 8 November 2006 at 05:22

Status:

Needs review

» Needs work

@drinkypoo could you submit a unified diff patch described under http://drupal.org/diffandpatch? I'll take a look once that is up. Thanks.

Comment #6

hyperlogos commented 8 November 2006 at 20:12

Status	File	Size
new	img_assist-alignment-and-origsize_2.patch	4.06 KB

Here 'tis. I ended up patching the .css file in the bargain, might as well. (so this is a recursive unified diff, whee.)

Comment #7

onionweb commented 13 November 2006 at 18:30

I find the centering does not work.

However. if instead of

+  if ( $attributes['align'] == "center" ) {
+    $output = "<span class=\"inline {$attributes['align']}\" style=\"width: {$attributes['width']}px;\">";
+  } else {
+    $output  = "<span class=\"inline {$attributes['align']}\">";
+  }

You leave that bit the way it was, and add "text-align:center" to the span.center class, plus

change


 $output .= "<span class=\"caption\" style=\"width: {$width}px;\">$caption</span>";

to

    if($attributes['align']=='center'){
    $output .= "<span class=\"caption\">$caption</span>";
    }else{
    $output .= "<span class=\"caption\" style=\"width: {$width}px;\">$caption</span>";
    }

everything works.

Comment #8

hyperlogos commented 13 November 2006 at 20:02

text-align: center; is not supposed to center anything but text. It was originally implemented to behave this way in IE (official suicide bomber of the ACID2 test) because CSS centering is hard. If Moz does it now it's only to try to achieve parity with IE, but it's still not compliant CSS.

It works properly here; maybe we just need a different name for the style to avoid a collision? Is there a standard drupal style for centering? I'll set up a new test site and test out the patch on a plain vanilla install to see if it works there.