I've had a look at this patch (3d38a7d). It works well for me, and it is certainly a welcome improvement.
I have a few comments. Issue ✨ Use checkbox and radio instead of Select, Provide better user experience Needs review (patch 04821ad) includes the same functionality, and a bit more, and also works well. I've compared the two approaches.
#3311345 not only sorts terms alphabetically, and provides check boxes on the page for selecting terms to be merged, it also provides check boxes on the following page for selecting the term to be merged into.
#3311345 also provides hyperlinks to the terms (on both pages), which is handy for checking that you have selected the correct term(s).
These are both worthwhile benefits which this patch doesn't have.
This patch updates the form definition (#empty_option to #description), ensuring that the help text gets displayed. #3311345 doesn't do this, and fails to display the help text.
The help text is not modified by this patch, but I think it should be, because it is misleading. It says "Select two or more terms to merge together". If I am wanting to merge one term into another existing term, then I should only select a single term on this form. The help text should instead say "Select one or more terms to merge". I think that this clarification of the help text can legitimately come within the scope of "provide a better UI".
This patch introduces the use of Drupal\Core\Render\Element\Checkboxes::getCheckedCheckboxes to determine which boxes have been checked. #3311345 appears to function perfectly well without this. I'm wondering why this extra call was introduced? What are the benefits?
In summary, I suggest combining (merging!) this issue with #3311345 to obtain the benefits of both.
- 🇺🇸United States DamienMcKenna NH, USA
@dabley: Given you've committed this change should this issue be closed and additional work be done is separate issues?
Hi @DamienMcKenna, the changes I committed were in a different branch (3311345-use-checkbox-and). I suggested that those changes could be merged into the branch for this issue (3280163-ui-improvements). I was waiting to hear from other people whether they thought this would be a good idea or not.
This issue seems to have already made it's way into release 2.0.0-beta4, (while #3311345 has not), so it would probably be better to close 3311345 and mark it as a duplicate of this one.
(I'm still a beginner in terms of Drupal contributions, so I'm not sure of the best way of doing things.)