At the moment most* fields require 'click sortable' => TRUE in their data. This is definitely more common than not wanting a click sortable column. So if we flip this round we can just specify 'click sortable' => FALSE where we need to.

I have put this logic in the click_sortable method currently, but maybe we could do this in init() instead?

I may have missed a few instances where we need to specify FALSE.

CommentFileSizeAuthor
#6 1891036-6.patch24.86 KBdamiankloip
#6 interdiff.txt2.33 KBdamiankloip
#2 1891036-2.patch23.2 KBdamiankloip
#2 interdiff.txt378 bytesdamiankloip
d8.click-sortable.patch23.39 KBdamiankloip

Comments

dawehner’s picture

dawehner
Primary language German
commented 
+++ b/core/modules/file/file.views.incundefined
@@ -114,7 +110,6 @@ function file_views_data() {
-      'click sortable' => FALSE,

Ups ... feels wrong.

If we put this logic into the init() method it seems to be harder to understand. In click_sort() it's directly written down.

damiankloip’s picture

damiankloip commented
StatusFileSize
new interdiff.txt378 bytes
new 1891036-2.patch23.2 KB

Good point, the extension plugin doesn't benefit from that.

dawehner’s picture

dawehner
Primary language German
commented

Sorry.

+++ b/core/modules/system/tests/modules/entity_test/entity_test.views.incundefined
@@ -45,6 +45,7 @@ function entity_test_views_data() {
+      'click sortable' => FALSE,

:)

dawehner’s picture

dawehner
Primary language German
commented

Totally forgot, shouldn't we actually write tests for the new default behavior?

damiankloip’s picture

damiankloip commented
Assigned: Unassigned » damiankloip
Status: Needs review » Needs work

#3 That's what the issue is for :) I knew there would be some mistakes.

#4 Yeah, definitely. I will work on this today.

damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review
StatusFileSize
new interdiff.txt2.33 KB
new 1891036-6.patch24.86 KB

I think having the uuid column as click sortable => FALSE is correct? We don't want that to be sortable.. do we?

Here is a new patch with tests, lucky we have them, because I spelt definitions wrong ;)

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Awesome!

catch’s picture

catch
he/him
Primary language English
commented
Title: Make 'click sortable' default to TRUE » Change notice: Make 'click sortable' default to TRUE
Priority: Normal » Critical
Status: Reviewed & tested by the community » Active
Issue tags: +Needs change record

Committed/pushed to 8.x, thanks! Will need a change notice.

dawehner’s picture

dawehner
Primary language German
commented
Status: Active » Reviewed & tested by the community

http://drupal.org/node/1895020

catch’s picture

catch
he/him
Primary language English
commented
Title: Change notice: Make 'click sortable' default to TRUE » Make 'click sortable' default to TRUE
Priority: Critical » Normal
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs change record

Yep.

Automatically closed -- issue fixed for 2 weeks with no activity.

idebr’s picture

idebr commented
Issue summary: View changes

jhedstrom identified a regression in Views where fields are no longer sortable over at #2395763: Fields are not 'click sortable' in views.