- 🇺🇸United States robbt
I attempted to apply this patch using composer and the file wasn't created.
When I manually created the file based upon the patch I could indeed click sortable and sort the table based upon the link.
But one issue I noticed is that it sorts based upon the URL rather than the display name which probably isn't ideal for everyone.
I'm also not sure if the best way to accomplish this fix is through using a hook_alter for views rather than fixing the links module directly, but that's going to require more research on my part to determine how other fields tell views they are sortable.
- 🇮🇳India Akhil Babu Chengannur
Applied patch from #16 and it works in both drupal 10 and 11 versions.
@robbit, Link field can be sorted based on title by changing Column used for click sorting friom URL to title.
I also checked core to see how other fields have enabled click sorting
Modules such as statistics, dblog etc usehook_views_data()
to define fields and add 'click sortable' parameter from there.Node module uses views_data handler "views_data" = "Drupal\node\NodeViewsData" for node entity and 'click sortable' added to fields from NodeViewsData.
taxonomy users hook_views_data_alter() to add some fields and click sorting is added from there.
- 🇮🇳India Akhil Babu Chengannur
Used by Style: Table to determine the actual column to click sort the field on. The default is usually fine.
This is the default description added to click sort column field.
// No need to ask the user anything if the field has only one column. if (count($field->getColumns()) == 1) { $form['click_sort_column'] = [ '#type' => 'value', '#value' => $column_names[0] ?? '', ]; } else { $form['click_sort_column'] = [ '#type' => 'select', '#title' => $this->t('Column used for click sorting'), '#options' => array_combine($column_names, $column_names), '#default_value' => $this->options['click_sort_column'], '#description' => $this->t('Used by Style: Table to determine the actual column to click sort the field on. The default is usually fine.'), ]; }
Maybe we can exclude "Used by Style:" and add only "Table to determine the actual column to click sort the field on. The default is usually fine" as the description?
- Status changed to Needs review
11 months ago 12:33pm 14 December 2023 - 🇺🇸United States robbt
So I was able to get this to patch to apply. I realized I needed to set patch level to 2 as described here - https://www.drupal.org/docs/develop/using-composer/manage-dependencies#s... → in order to apply a patch to core using composer-patches.
With that working I do think it makes sense to alter the description to say Used by Format: Table rather than Used by Style: Table which I think was from a previous version of Views.
I've created a new patch based upon 11.x that makes this change as well, so if anyone can test and review it then I think we can change it to RTBC as @akhil_babu suggests. My change merely updates the description and is based otherwise entirely on the patch on #16
- last update
11 months ago Custom Commands Failed - last update
11 months ago Custom Commands Failed - last update
11 months ago Custom Commands Failed - last update
11 months ago Custom Commands Failed - last update
11 months ago Custom Commands Failed - last update
11 months ago Custom Commands Failed - last update
11 months ago Custom Commands Failed - last update
11 months ago Custom Commands Failed - last update
11 months ago Custom Commands Failed - last update
11 months ago Custom Commands Failed - 🇺🇸United States robbt
The patch in #27 was failing drupal coding standard tests so I've added a new one that should at least pass the issue it was failing on (a lack of a <?php tag.
Otherwise if this passes the automated tests it should be ready for review.
- last update
11 months ago 30,727 pass - Status changed to Needs work
11 months ago 5:38pm 18 December 2023 - 🇺🇸United States smustgrave
From what I can tell the tests are check that the checkbox is there but doesn't test the sort?
Think we could also extend the tests to include the different column sort.
- 🇺🇸United States robbt
I think that testing the sort is a good idea. I haven't written any tests for a Drupal core contribution yet but I'm willing to give it a shot.
In looking at the code it appears that web/core/modules/views/tests/src/Functional/Handler/FieldWebTest.php has a test for testing click sorting that could be used as the basis for a new test of this functionality.Also I see web/core/modules/views/tests/src/FunctionalJavascript/ClickSortingAJAXTest.php which tests the same thing via AJAX.
In theory I think it would make sense to also test the functionality of sorting based upon the title (if it is enabled) and also based upon the uri (the default). I'm not sure how to mock this test up exactly but I'll be digging into other tests and reading the documentation to try to figure it out but any help or pointers in the right direction would be appreciated.
From a UI/UX perspective I would think that if a link field uses a title/display that would be the default sort chosen since that is what usually displays but that would require further changes to this code and probably views (beyond updating the description). I also don't know what sorting by the options would mean and if this even makes sense to include.