Note if someone applies this patch. The patch here also include "optimization fixes" and a "more verbose alt tag" which are parts of different bugs not relevant to this patch. Some of those have opposition to being applied.

I have a few issues with this patch as well.

What is with removing the sortable class? What if someone was using that to specially style sortable tables?

Why are you creating a new table inside of the cell headers? There has to be a better method of doing this with css, and if there isn't we shouldn't be doing this.
Have you even tried doing this with any extravagantly styled tables? It looks like if someone decided to use something fancy like outlines then styles would break.

mhorvath2161 wrote:

(In reply to comment #1)

Note if someone applies this patch. The patch here also include "optimization
fixes" and a "more verbose alt tag" which are parts of different bugs not
relevant to this patch. Some of those have opposition to being applied.
I have a few issues with this patch as well.
What is with removing the sortable class? What if someone was using that to
specially style sortable tables?
Why are you creating a new table inside of the cell headers? There has to be a
better method of doing this with css, and if there isn't we shouldn't be doing
this.
Have you even tried doing this with any extravagantly styled tables? It looks
like if someone decided to use something fancy like outlines then styles would
break.

The "sortable" class is only removed from the inner table generated by this script. It is left unchanged on the outer, original table. All other classes/styles get copied to the new table.

I don't think there's a CSS solution that will work consistently across different browsers. The current version was tested by another user (see: http://en.wikipedia.org/wiki/User_talk:SharkD/Sandbox/wikibits_4) across several browsers and shown to work. I also did some proof-of-concept testing using browserhots.org, and it worked in 52 of 54 browsers tested.

Sorry about the other style changes that slipped in. I'll submit a new patch.

mhorvath2161 wrote:

unified diff file

Update, minus the changes to icon ALT text and FOR loop optimization.

attachment wikibits_icon_alignment.diff ignored as obsolete

(In reply to comment #2)

The "sortable" class is only removed from the inner table generated by this
script. It is left unchanged on the outer, original table. All other
classes/styles get copied to the new table.

Ya, I deleted half the comment after noticing you were inserting a table into the column. Initially it looked like you were recreating the entire table. Just missed that one.

I don't think there's a CSS solution that will work consistently across
different browsers. The current version was tested by another user (see:
http://en.wikipedia.org/wiki/User_talk:SharkD/Sandbox/wikibits_4) across
several browsers and shown to work. I also did some proof-of-concept testing
using browserhots.org, and it worked in 52 of 54 browsers tested.

I still don't really like the idea of tables. It's an ugly abuse of tables which the web is trying to move away from. I'd prefer something involving floats and using a spacer on the left to even things out if centered.

mhorvath2161 wrote:

Iw ould prefer using floats, too. But, I have been having issues with floats (described here: http://en.wikipedia.org/wiki/Wikipedia:Village_pump_(technical)#Template.2FCSS_help), and I don't think they're supported as widely by browsers.

I wouldn't say the Web has moved very far from using tables for layout purposes. Just look at the Main page of Wikipedia and see how many tables it uses. The technology really hasn't advanced far enough for there to be a suitable alternative.

Well, to me for now Wikipedia's front page is fine. They are positioning a small section of the page.

What I dislike is using tables to align small icons, that I do consider abuse.

And what browsers don't support floats? Last I checked it was CSS1.

The only problem with floats is that on centered things the header is pushed a bit off center.
Though now that I think about it, tables probably give the same issue.

And does this patch consider rtl languages? Aren't they supposed to have icons like these on the left?
A float could easily take this into consideration.

As for the off center issue. It should be possible to make something float on the opposite side to even things out.

mhorvath2161 wrote:

Yes, floats are CSS1. But, there are differences in how browsers handle them. A problem arises when the text is wider than the cell. In IE, the result is to break the text where it comes into contact with the floated content. Firefox breaks the entire text onto a new line.

IE: http://img117.imageshack.us/img117/2406/wikipediatablefloatbreaqj9.png

Firefox: http://img117.imageshack.us/img117/7079/wikipediatablefloatbreatk2.png

There are also problems with vertical alignement. When the text spans multiple lines, the floated content is aligned with the first row of text. This means that the content can't be vertically centered. If multiple lines of text exist, the floated content will be aligned with the first row.

See: http://img117.imageshack.us/img117/4795/wikipediatablefloatbreacm3.png

I'm also not sure what relevance the direction a language is written in has to a user interface element. User interface and text rendering are separate considerations. That said, the table could just as easily be configured to place the icon on the left depending on the language.

ayg wrote:

Could you post screenshots of the problem the patch fixes, how it looks before and after the patch? I don't know what problem this is trying to solve. From the verbal description I have to ask whether this couldn't be fixed just by using non-breaking spaces.

mhorvath2161 wrote:

No problem.

Here's a screenshot of the old version:

http://img75.imageshack.us/img75/4307/wikipediatablesortableoig3.png

Notice how the arrow icons appear in all sorts of odd places. Sometimes they break onto lines all by themselves.

Here's a screenshot of the changed version:

http://img75.imageshack.us/img75/3145/wikipediatablesortablenyz1.png

The position of the icons is fixed. They always appear in the same place in a cell.

mhorvath2161 wrote:

unified diff file

Update. I've simplified the code to some degree, made the style more compact and fixed a bug I introduced while copying from my working, cumulative version.

Here's what this latest version looks like:

http://img296.imageshack.us/img296/5610/wikipediatablesortablefrc5.png

attachment wikibits_icon_alignment.diff ignored as obsolete

mhorvath2161 wrote:

unified diff file

That might have been *too* compact, sorry.

attachment wikibits_icon_alignment.diff ignored as obsolete

ayg wrote:

I see. The way I'd be inclined to solve this is using absolute positioning. This CSS more or less works for me:

table.sortable th {
margin-right: 16px;
position: relative;
display: block;
}
.sortarrow {
position: absolute;
right: 2px;
top: 50%;
}
.sortarrow img {
position: relative;
top: -5px;
}

In practice, we would want to add a wrapper div inside the header cells and use that for the first set of rules, instead of assuming table.sortable th will do what we want, but this should be good enough for demonstration purposes. Also, this needs to be tested on various browsers, but I'm pretty sure it will work reliably, since it's just CSS1. Although one never knows with IE6, which might require a workaround.

mhorvath2161 wrote:

Could you create a demonstration file? In my experience, the browsers tend to be kind of finicky when mixing abeolute with relative positioning. Tables, on the other hand, have had much better support for a much longer time.

ayg wrote:

Just put that CSS in your user CSS on Wikipedia or something.

mhorvath2161 wrote:

This is what it looks like in Firefox:

http://img383.imageshack.us/img383/7273/wikipediatablesortablecto5.png

This is what I mean when I say that support for "CSS solutions" is sketchy. Tables are tried and true and work with even very old browsers.

mhorvath2161 wrote:

(In reply to comment #15)

This is what it looks like in Firefox:
http://img383.imageshack.us/img383/7273/wikipediatablesortablecto5.png
This is what I mean when I say that support for "CSS solutions" is sketchy.
Tables are tried and true and work with even very old browsers.

FYI, the Firefox behavior is the *correct* behavior. It's IE that is acting screwy in this case.

ayg wrote:

Yeah, okay, so you really do have to add a wrapper div. (I was testing with a one-column table, so I didn't see that.) No big deal, it's still a lot smaller than your solution. A quick first attempt on my part didn't work, probably because something is making assumptions about how many levels deep something is being nested.

Anyway, I'm not going to commit a solution using this table hack, I'd like to see this as proper CSS. We use tables more than enough already for layout in MediaWiki.

mhorvath2161 wrote:

demonstration file

I found a CSS solution that works well in the majority of browsers. I've attached two demonstration files showing the CSS and table methods in action. Using browsershots.org I tested the files in various browsers. 50 browsers were tested.

The CSS method didn't work in:
IE4
IE5.5
IE6
IE8
Dillo

The table method didn't work in:
IE4
IE8
Dillo

IE4 can be safely ignored as it's so ancient. IE8 is still in beta so things will probably change in due time. Dillo is a text-only browser I think, so it can be ignored as well. IE5.5 and IE6 are a problem though.

Attached:

mhorvath2161 wrote:

unified diff file

OK, I got it to work in IE5.5 and IE6! I've attached a patch. In addition to the patch, the following changes need to be made to the stylesheet:

.sortheader {
margin-right: 13px;
position: relative;
display: inline-block;
}
.sortarrow {
position: absolute;
right: -13px;
top: 50%;
}
.sortarrow img {
width:12px;
height:14px;
position: relative;
top: -6px;
}

attachment wikibits_icon_alignment.diff ignored as obsolete

mhorvath2161 wrote:

unified diff file

Woops. I made a few too many changes that broke the rest of the code. Here's a new patch minus those additional changes.

attachment wikibits_icon_alignment.diff ignored as obsolete

ayg wrote:

Slightly fixed-up version of previous patch, rebased to recent trunk

(Term is over and I can get back to reviewing patches. I didn't forget about this.)

This is the same patch but with the following changes:

1. Works with current trunk, after r44879 (which I committed specifically so this patch would be readable).

2. Removed border="none" on the image, which seems unnecessary. We already do this in CSS.

3. Adjusted one extra line to account for the extra div. Without this extra line, actually clicking the icon fails.

I haven't tested in browsers other than Firefox 3 yet. display: inline-block looks kind of weird . . .

Attached:

ayg wrote:

Okay, so this doesn't work in IE5 (but it does in IE5.5), and doesn't as recently as Opera 9.23, a 2007 release (it works in Opera 9.5). It works on Firefox since 1.0, and Safari since at least 3.0 (didn't test earlier). I don't know if this slight aesthetic improvement is worth breaking compatibility with those browsers.

"Align the sorting icon so that it always remains in a fixed position and
doesn't break onto new lines."
Fixed in r86088