Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Currently the results are displayed in the order in which they are in the database. Although only 10 are shown, the time taken to choose a specific result can be decreased if the list is in alphabetical order.
Comment | File | Size | Author |
---|---|---|---|
#19 | code-cleanup.patch | 9.82 KB | bleen |
#19 | 1530026.patch | 12.12 KB | bleen |
#17 | 1530026.patch | 20.29 KB | bleen |
#15 | 1530026.patch | 20.2 KB | bleen |
#9 | 1530026.patch | 20.2 KB | bleen |
Comments
Comment #1
hass CreditAttribution: hass commentedShould be default
Comment #2
bleen CreditAttribution: bleen commentedThis should do it ... creates an "Order" select list on the widget form ...
(I also snuck in a bit of code cleanup and a couple "todo's".... shhhh dont tell anyone)
Comment #3
hass CreditAttribution: hass commentedThis looks wrong logic to me. If you sort parts of the query result it's too late. You may miss titles... I believe the query needs order by... Isn't it? I may be wrong as I have not tested the patch, this is all code wise.
Comment #4
bleen CreditAttribution: bleen commentedNot all of the autocomplete widgets use a db query so I chose to simply sort the final array that is created regardless of the source... Would be great if you could try out the patch and post results.
:)
Comment #5
mrtoner CreditAttribution: mrtoner commentedI'll check this out in a bit. Sorting the results is what I expected (and that's what ORDER BY does, BTW, at least for MySQL). I've never seen an autocomplete that didn't sort the results in ascending alphabetical order, though, so the sorting option seems unnecessary and an example of preference creep.
Comment #6
mrtoner CreditAttribution: mrtoner commentedSince I'm just learning Git it's likely I just made a mistake, but I'm seeing two hunks that are rejected in the patch.
Comment #7
hass CreditAttribution: hass commentedBut it cannot work reliable if the sql request is not ordered by name. No need to test, it cannot work correctly. In your example we may need to sort both, sql and non-sql.
Comment #8
bleen CreditAttribution: bleen commentedRe #5: imagine the case where someone is providing suggestion (one of the types of ac widgets available) in that case they may have been explicit in ordering the suggestions or they may not have and they want them ordered alphabetically ... I agree that in the case of a user who is using the "existing field data" that most of the time they will want everything sorted. In this case there is no extra work for the user since the default setting is sort ASC.
Re #6: I know what I did wrong ... I'll put up a new patch when I get to work
Re #7: I see what you are saying now and I think you are correct: I will need to add the order by to the SQL as well
Comment #9
bleen CreditAttribution: bleen commentedLets see how we do with this patch
Comment #10
mrtoner CreditAttribution: mrtoner commented@bleen18: Ah, yes, that's right.
@hass: I'm not sure I understand why the ORDER BY needs to be in the query. ORDER BY doesn't sort the column before the SELECT is performed, it sorts the results that are returned. So ORDER BY shouldn't affect which rows are returned.
Comment #11
mrtoner CreditAttribution: mrtoner commentedPatch applies, but I'm getting an error now:
Comment #12
mrtoner CreditAttribution: mrtoner commentedBTW, "A to Z"/"Z to A" may not always be appropriate: the user may be using numerical fields. I suggest "Ascending," "Descending," and "None."
Comment #13
bleen CreditAttribution: bleen commented#10: Its because there might be 20 possible results but we are only limiting to 10. In that case we want to have MYSQL orderby first and then limit to 10.
#11: Huh?? The error there is this: ORDER BY ASC ASC
But I have no idea why that might be happening. My brain be hurtin. Ill play a bit though.
#12: Agreed
Comment #14
mrtoner CreditAttribution: mrtoner commentedOh, right: LIMIT returns the first 10 from the unordered results without ORDER BY. With "contains" matching that probably doesn't make a difference, but with "starts with" matching you'll miss a lot more relevant matches.
Umm:
$select->orderBy($order);
Doesn't that just insert "ORDER BY ASC"? It needs to order by the column:
$select->orderBy($col, $order);
Comment #15
bleen CreditAttribution: bleen commentedDOH!!!!! Of course you need a column. And when I did my spot check I used "suggested".
How bout this patch
Comment #16
mrtoner CreditAttribution: mrtoner commentedNo change #15 from #9.
Comment #17
bleen CreditAttribution: bleen commentedOoops ... uploaded the old file. Sorry bout that
Comment #18
mrtoner CreditAttribution: mrtoner commentedDing, ding, ding! We have a winner.
Comment #19
bleen CreditAttribution: bleen commentedThanks for your help with this mrtoner & hass...
I committed both of the patches below. One was the basic code cleanup (apply first), the other handled the actual reordering (apply second)