- 🇺🇸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
over 1 year 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
over 1 year ago Custom Commands Failed - last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago Custom Commands Failed - last update
over 1 year ago Custom Commands Failed - last update
over 1 year 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
over 1 year ago 30,727 pass - Status changed to Needs work
over 1 year 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.
- 🇺🇸United States robbt
I'm here at DrupalCon Atlanta in the mentored contribute @partika and we looked under the core directory and don't see any other fields that are have their 'click sortable' tested. I believe that whether a field gets sorted or not is actually handled by Views code and so adding the ability to sort links shouldn't require a test of the sort. Going to push the code and update this to a MR.
- Merge request !11662Issue #3239705 "Add the ability to sort links via Views by using _views_data_alter in a link.views.inc file to enable the ability to sort" → (Open) created by robbt
- 🇺🇸United States robbt
This is currently not working because it uses the deprecated .views.inc approach that was removed from D11. It needs to be refactored to use ModuleViewsData.php to extend EntityViewsData in order to set click sortable to true. Although typically click sortable is enabled by default.
- 🇺🇸United States robbt
It looks like the appropriate thing to do for D11 compatibility is to change this to a views hook - see the commit https://git.drupalcode.org/project/drupal/-/commit/1da8aa097417905a9772d... and this issue https://www.drupal.org/project/drupal/issues/3489415 📌 Deprecate views_field_default_views_data Active
Essentially just need to rewrite the .views.inc to be compatible with the specific hook type.
Like for instance https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/taxon... replaced https://git.drupalcode.org/project/drupal/-/blob/10.5.x/core/modules/tax...