[Views] Table sort class & indicator for active field is wrong.

The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

This is really funny... this bug seems to exist since Drupal 6 and the argumentation is

It's not wrong. It's saying what will happen when you click on it. Not what is currently happening.

( #552752: hover text and file names are incorrect for arrows indicating ascending and descending sorts on table columns → )

It's true but well, everyone is doing it the other way around and that's typically what people are used to ;)
I can't believe Drupal is doing this differently from all other software sorting UI's I know! :D

Some for reference:

• Windows Explorer
• https://datatables.net/
• https://handsontable.com/demo
• http://swimlane.github.io/ngx-datatable/
• https://primevue.org/datatable/#multiple_sort
• https://vuetifyjs.com/en/components/data-tables/basics/#server-side-tables

Just count the stars... what else has to happen to accept it's the wrong symbol in Drupal, indicating the current sorting order? ;)

• If the arrow is going up on a sorted row, the results are expected to be sorted ascending
• If the arrow is going down on a sorted row, the results are expected to be sorted descending

These screenshots are from claro, but it's the same in Gin for example:

(simplytest.me Umami Demo)

The icons are already correct, just the classes are the wrong way around:

• tablesort tablesort--desc => tablesort tablesort--asc
• tablesort tablesort--asc => tablesort tablesort--desc

Ok here we go.

I'd be interested why someone wrote this curious line without even leaving a comment on this crazy confusion:

$context['sort'] = (($context['sort'] == self::ASC) ? self::DESC : self::ASC);

The Needs Review Queue Bot → tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request → . Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

Forgot to create the MR from the issue fork, sorry! Hiding the outdated patches.

Recommend using standard issue template.

Also probably will need test coverage or least an additional assertion to an existing test.

How about what I wrote up for #3266019: Tablesort indicator class direction is backwards. → ?

Thanks @darvanen! I didn't see that one yet!

I copied your fix into a separate MR over here against 11.x and to also make it testable manually using the tugboat link!

My MR meanwhile doesn't seem to fix the issue. Perhaps we need both fixes or just the one in views?

Could you also have a look at #25 please? Similar strange code can be found in views here, near your change:
https://git.drupalcode.org/issue/drupal-2853130/-/blob/c291bb2dafe715cd8...

We should next find out:
1. Is it a views bug or a claro bug? (The svg's & icons seem correct as a starting point) You already gave some information on that
2. At which places does it have to be corrected (see the lines above here)
3. Why these crazy code lines in #25 and above?

So we can decide how to fix and test it properly.

I agree with @smustgrave a issue summary cleanup is also needed, or should we instead reopen your issue (if it's a views bug and not a general / claro bug!)

I'm inclined to think it's a views bug not a claro bug, but the effects are far-reaching and I think would consitute a breaking change with no BC layer possibility as far as I can see.

Let's keep it in here where everyone else is, that other issue was just me until I found this, we can always change the issue name :)

As for #25 I suspect the reasoning at the time had something to do with turning the current state into an action and thus the reverse of what is provided. I also think it's silly but hindsight is 20:20 and it was written a long time ago, I'd say before a standard emerged and was widely known. I guess the question is "what now"?

I'd love to get a subsystem maintainer's read on how bad it would be to flip that logic. I think we would have to set up follow-ups in all themes that support Drupal 10 😬

@darvanen +1 on that! Totally agree!
@smustgrave how can we best get a decision here, before wasting time?

I'll ask in #bugsmash, this fits in our remit and one or two of the views maintainers is in the channel.

Copied the IS out of the closed issue as a start to updating it.

@darvanen any feedback there? I'd really like to get this done, it's just confusing.

Oh! I tagged you in the conversation and with no reply I figured you moved on to other things (which of course is fine, everyone is busy). Would you like me to tag you again? I'm afraid I can't transcribe the conversation to here very well, don't know how others get such good exports.

@darvanen sorry I'm not on Slack (and already too many other things to do... so won't join soon), but you may flag my team mate @Grevil to take a look.

But in general, it would be great to have the outcome and next steps documented here for others?

I'll tag @grevil and see if I can find out how to get a decent export.

I haven't been able to get an export yet but now I've had a little more time I actually read the thread and can summarise: @larowlan suggested this is an excellent use-case for ✨ Add an API for feature flags Active and I agree. The conversation from that point was more about feature flags in general.

It sucks to postpone something based on a feature request but if we get enough momentum in that issue it shouldn't take too long considering a core committer is behind the feature request.

What do you think?

Thank you @darvanen, well my 2 cents are: Let's just get this fixed. This is wrong since ever and against all logics and best practices. Adding more complexity won't help getting things done soon.

This is a bug and not a feature and it's not called bug flags :P

(To read with a sense of humor). I don't see any reason not to fix this asap.