JamesPosko → created an issue.
I think the MR 25 based on branch 3356663-tests-no-table-drag-row-change in this same issue fork is showing the 3 new test failing. The branch only contains the tests and not the global JavaScript update. Is this how a test-only set should be created?
Added a test to run on menu Administration edit page and one to run on views Content filter rearrange UI dialog along with the first test on the manage form display.
There is a new branch that has the new test but not the update to the global JavaScript and the tests fail.
Sounds good! Thank you!
So remove the tests from MR23 but keep the change to use .draggable
, correct?
Create a new branch in this issue fork to apply the tests in a different MR?
I'll take a look at a few of the different core admin pages and add test for the different variants (tree / flat).
Would a test-only patch mean to create a patch that contained the tests that can be applied to the feature branch / merge request?
Ran the tests again and they all passed this time. I'm not sure what happened on the first run but after going through some of the documentation I thought the first step would be to run the test again.
Does the new test added meet the expectations on the core table drag functionality?
What are the other Drupal core exclusions needed?
I added a test to the global functional tests to check that choices isn't applied to the manage form display table drag selects. I'm not sure why the test for the choices widget failed, all tests are passing on my local development environment with the same versions.
Since changing to the draggable exclusion, table drag is working on core menus, manage form display, manage display, views, roles, and blocks. The update is working on contrib paragraphs and the Search API as well.
I haven't created an automated test but will learn and try.
I can create the follow-up issue feature request to add a custom exclude setting.
Agreed. There doesn't seem to be a standard way the attribute is being implemented. Though, all these selects are part of the table drag implementation and the new exclude of .draggable should exclude all these selectors.
Weight, parent and region selects are mostly used with the table drag interface. I think the previously existing selectors can be replaced by .draggable.
I came across another instance where the current ends with selector isn't matching a select. A new attribute selector that contains "weights" would need to be excluded. In the Search API manage processors admin config for processor order uses table drag with weight selects but the attribute uses "weights" instead of "weight" (data-drupal-selector="edit-processors-number-field-boost-weights-preprocess-index").
I think the exclusion of .draggable should replace the need to search for the other two selectors that are currently being excluded. None of the select elements in the table drag row have the Choices.js applied. The single exclusion of .draggable seems to work when rearranging table drag elements on paragraphs, menus, blocks, roles, and views. It is a good question if another core JS feature might use weight or parent?
@Anybody I'm glad to hear this can still work as is with the 2.x branch. Thank you for making the MR from the 2.x branch in this issue fork and for your support.
The MR contains the proposed change to target the parent node with the class .draggable
and not apply choices.js to the child select elements. The code comment has been updated as well.
I need a bit of help with the issue fork and creating the merge request. I'm following the documentation on the Drupal site but I ended up pushing the changes to the 2.x branch and not the 3356663-table-drag-row branch in the issue fork. How should I proceed with the merge request? Thank you for your help as I learn and work through the process for Drupal projects.
JamesPosko → created an issue.
Thank you @Anybody for the assistance and collaboration on this issue. Thank you @Grevil for reviewing and testing.
I went with using select[data-drupal-selector$="weight"]
because the name
attribute had a few different version of ending with weight with square brackets. The classes on the select element are all different when table drag functionality is added. I attempted to use offsetParent
but if the element or any ancestor has the display
property set to none
it will return null
.
I tested this on Paragraphs, menus, blocks, roles, and views rearrange table drag elements.
I'll give adjusting the selectors a try locally and update the merge request. Thanks for collaborating on possible solutions.
Wow! This is great! Thank you for the quick response.
I was able to clone the repository and test the merge request changes. The merge request works great on the menu table drag elements.
Thinking about your last question, I tested the update with Paragraphs and the table drag elements use a different class on these elements. This produces the same issue as before. So the name$=['weight']
(ends with) selector might be a better way to catch the various instances of elements with the table drag functionality. Or maybe a way to skip the select elements with a parent that has a class of .tabledrag-hide
.