Sortable tables: Force sortorder of a column
Closed, ResolvedPublic

Description

Author: mhorvath2161

Description:
unified diff file

This patch adds the ability to force a particular sortorder of a column instead of relying on the script to detect the proper sortorder. This is achieved by adding a "sortorder" attribute to the column header, and specifying the desired sortorder as the value of this attribute. The actual options, however, need to be updated to be current with the other patches. The ones included in the diff can be considered examples used more for demonstration purposes.


Version: unspecified
Severity: enhancement
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=31332

attachment wikibits_force_sortorder.diff ignored as obsolete

bzimport added a project: MediaWiki-JavaScript.Via ConduitNov 21 2014, 10:22 PM
bzimport added a subscriber: wikibugs-l.
bzimport set Reference to bz15406.
bzimport created this task.Via LegacySep 1 2008, 2:58 AM
DanielFriesen added a comment.Via ConduitSep 1 2008, 8:28 AM

sortorder is not a valid XHTML attribute. Adding this patch will go against our attempts at keeping MediaWiki XHTML valid.

I recommend finding a way to do this using classes rather than invalid XHTML attributes.

bzimport added a comment.Via ConduitSep 1 2008, 2:41 PM

mhorvath2161 wrote:

(In reply to comment #1)

sortorder is not a valid XHTML attribute. Adding this patch will go against our
attempts at keeping MediaWiki XHTML valid.
I recommend finding a way to do this using classes rather than invalid XHTML
attributes.

The proposal could certainly be changed to use classes. However, the code already uses a non standard XHTML attribute, "sortdir". Should this be changed as well?

DanielFriesen added a comment.Via ConduitSep 1 2008, 2:54 PM

Sortdir is an attribute set internally by the js when you click on the sort button. It's ok for JS to set non-standard attributes.

There is another proposal for a default reverse sort, that patch should probably make sure to be one that sets that attribute.

bzimport added a comment.Via ConduitSep 1 2008, 3:58 PM

mhorvath2161 wrote:

It's silly to say that non-standard attributes are supported when added by JavaScript, but not otherwise. You're confusing being able to pass a verification test with the issue behind the proviso.

bzimport added a comment.Via ConduitSep 1 2008, 7:58 PM

ayg wrote:

The sortdir thing should be changed too. Frankly I don't see why authors shouldn't be allowed to make up their own site-specific attributes. It's handy sometimes. But we want the trendy Standards-Compliant label, I suppose, so we have to live with it.

There's no way, in any event, for users to add a custom attribute in wikitext. Something like this:

{| class="sortable"

-

! sortorder="numeric" | This is the header
...

}

will just have the "sortorder" attribute stripped as invalid. So I'm not clear on how this is supposed to be used, even if we did want to break XHTML validity, which we don't.

DanielFriesen added a comment.Via ConduitSep 1 2008, 8:04 PM

It's only used internally. Attributes in the code are commonly used for storing variables related to a node. In this case we store the current sort direction so that we can reverse it on the next sort.

I can't remember if we have a class to make the list initially sorted... If we don't it would be good. Either way a reversesort css class to make this sort in the opposite direction by default would be nice.

bzimport added a comment.Via ConduitSep 2 2008, 1:05 AM

mhorvath2161 wrote:

I took a look at the code to see how easy it would be to accomplish this using only class names. While it could be done, it would require supporting a separate classname for each sorting function. As the number of sorting functions increases, so does the number of classes you have to check for (as opposed to checking for a single, uniquely named attribute). I'm worried that the large number of classes will negatively affect the speed of the script.

DanielFriesen added a comment.Via ConduitSep 2 2008, 10:28 AM

You don't have to check for a pile of stuff, just a pattern.

var sortorder

DanielFriesen added a comment.Via ConduitSep 2 2008, 10:47 AM

Ack, wtf, I write a section of JS code, and bugzilla deletes it. Bah...

Well, really you'd just break up the className by \s and then test for any classNames that start with something like sortorder-

bzimport added a comment.Via ConduitSep 2 2008, 5:59 PM

mhorvath2161 wrote:

You mean, like:

"sortorder-numeric"
"sortorder-alphanumeric"
"sortorder-date1"
...

I suppose that would work.

bzimport added a comment.Via ConduitSep 2 2008, 10:45 PM

mhorvath2161 wrote:

unified diff file

I created an alternate version that uses class names instead of attributes. Seems to work OK.

attachment wikibits_force_sortorder_b.diff ignored as obsolete

bzimport added a comment.Via ConduitSep 2 2008, 11:18 PM

mhorvath2161 wrote:

Actually, "sortorder-XXXX" could be truncated to "sort-XXXX" without any problems.

bzimport added a comment.Via ConduitSep 2 2008, 11:34 PM

mhorvath2161 wrote:

unified diff file

"sortorder-xxxx" classes renamed to "sort-xxxx".

Attached: wikibits_force_sortorder_b.diff

bzimport added a comment.Via ConduitJan 21 2009, 7:21 PM

ssanbeg wrote:

Should the regex be anchored? I think something like
var sortClass = ('' + td.className + '').match(/^sort-\w+$/);

would be more appropriate

bzimport added a comment.Via ConduitJan 21 2009, 7:23 PM

ssanbeg wrote:

(In reply to comment #14)

Should the regex be anchored? I think something like

var sortClass = ('' + td.className + '').match(/^sort-\w+$/);

would be more appropriate

Oops, that would probably break multiple classes....

bzimport added a comment.Via ConduitJan 22 2009, 10:43 AM

mhorvath2161 wrote:

(In reply to comment #15)

(In reply to comment #14)
> Should the regex be anchored? I think something like
> var sortClass = ('' + td.className + '').match(/^sort-\w+$/);
>
> would be more appropriate
>
Oops, that would probably break multiple classes....

You just made me notice that the quotes are supposed to have spaces in them. I.e.

  • var sortClass = ('' + td.className + '').match(/(sort\-\w+)/);

+ var sortClass = (' ' + td.className + ' ').match(/(sort\-\w+)/);

-Mike

bzimport added a comment.Via ConduitJan 23 2009, 8:52 PM

ssanbeg wrote:

(In reply to comment #16)

(In reply to comment #15)
> (In reply to comment #14)
> > Should the regex be anchored? I think something like
> > var sortClass = ('' + td.className + '').match(/^sort-\w+$/);
> >
> > would be more appropriate
> >
> Oops, that would probably break multiple classes....

You just made me notice that the quotes are supposed to have spaces in them.
I.e.

  • var sortClass = ('' + td.className + '').match(/(sort\-\w+)/); + var sortClass = (' ' + td.className + ' ').match(/(sort\-\w+)/);

    -Mike

Then should the regex have the spaces, too? i.e.

var sortClass = (' ' + td.className + ' ').match(/ (sort\-\w+) /);

It seems like that should avoid false positives in the match.

bzimport added a comment.Via ConduitJan 24 2009, 3:43 PM

david.potter wrote:

*** Bug 13535 has been marked as a duplicate of this bug. ***

bzimport added a comment.Via ConduitJan 26 2009, 9:18 PM

mhorvath2161 wrote:

(In reply to comment #17)

Then should the regex have the spaces, too? i.e.

var sortClass = (' ' + td.className + ' ').match(/ (sort\-\w+) /);

It seems like that should avoid false positives in the match.

I suppose it can't hurt. I don't think false positives are very likely though.

-Mike

bzimport added a comment.Via ConduitDec 23 2009, 8:22 PM

bluehairedlawyer wrote:

How come this has stalled for so long?

DieBuche added a comment.Via ConduitDec 20 2010, 12:08 AM

will be fixed in the jQuery implementation

DieBuche added a comment.Via ConduitJan 8 2011, 3:18 PM

fixed in patch for Bug 8028

bzimport added a comment.Via ConduitJan 12 2011, 7:36 PM

mhorvath2161 wrote:

Quote: "How come this has stalled for so long?"

Because I got into an argument with the devs and they said they would no longer be looking at my requests.

DieBuche added a comment.Via ConduitApr 15 2011, 8:36 AM

r86088

brion added a comment.Via ConduitJun 22 2011, 8:02 PM

r86088 is seriously broken and may get reverted; reopening. Needs unit tests (see initial basic unit tests in r90595 which show up errors; will also need tests for the particular case this bug is about.)

DieBuche added a comment.Via ConduitJul 7 2011, 8:25 AM

reclosing

bzimport added a comment.Via ConduitNov 27 2011, 1:41 AM

zedlightman wrote:

This needs to be implemented soon. It is very difficult to follow all the rules in this and other sections of the Help:Sorting page. Even experienced table editors like myself have difficulty. There are so many rules, and they change at times:
*http://en.wikipedia.org/wiki/Help:Sorting#Numerical_sorting_problems

I have been updating Help:Sorting, and doing various tests. There are still some inexplicable bugs where a column follows all the rules for numerical sorting, and it still does not sort correctly. I have looked at r86088 comments, and I am glad there has been some serious work down.

Has work on this stalled? If you are working on this then please also incorporate my request to put the sorting icon above the text. This is very important. See my comment #15 here:
*https://bugzilla.wikimedia.org/show_bug.cgi?id=31755#c15

bzimport added a comment.Via ConduitSep 26 2012, 6:50 PM

zedlightman wrote:

See this related feature request:
Bug 24523 - "Make table sorting ignore refs when determining data type".

bzimport added a comment.Via ConduitSep 30 2012, 7:59 AM

zedlightman wrote:

This has been fixed in English Wikipedia according to discussion here:
*[[Help_talk:Sorting#HTML5]]
*[[Help_talk:Sorting#Documentation]]

data-sort-type plus a value forces a column to be sorted according to a specific method.

See also:
*[[Help:Sorting#Numerical_sorting_problems]]
*[[meta:Help:Sorting#Sort_modes]] - go to the subsection about forcing the sort mode for a column. It lists the values that can be used.

TheDJ added a comment.Via ConduitJul 20 2013, 8:49 PM

I think we can call this one closed now.

Add Comment