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.
Perhaps this request has been made before, but I was wondering: why don't we display the number of results (we're passing a SQL COUNT query to pager_query() anyway) as well as the range of results currently being displayed on the search results page.
Kind of like how google will say "Results 1 - 10 of about 76 for "blah blah".
-Ankur
Comment | File | Size | Author |
---|---|---|---|
#26 | A.patch | 8.71 KB | bcn |
#26 | B.patch | 10.31 KB | bcn |
#26 | before_after.gif | 80.91 KB | bcn |
#20 | 22627_theme_pager.patch | 8.36 KB | David Lesieur |
#18 | 22627_theme_pager.patch | 8.39 KB | David Lesieur |
Comments
Comment #1
Zen CreditAttribution: Zen commentedSeconded.
Comment #2
geodaniel CreditAttribution: geodaniel commentedThe attached patch adds '[total] results found for [query], showing page [current] of [max]' to the top of the search pages.
I attempted to get it to show actual result counts (eg. 1-10, 11-20, etc) per page but couldn't figure out how to intercept that information from between the search and pager functions. If anyone knows how to get that info, please update the patch, as showing result numbers is probably more useful than page numbers.
Comment #3
Steven CreditAttribution: Steven commentedThe patch is not escaping the search keys, leading to XSS issues.
You could argue that this is a theming issue though. The Civicspace theme changes theme_pager() so it already returns a summary like this patch.
Comment #4
catchComment #5
moshe weitzman CreditAttribution: moshe weitzman commentedsubscribe. would be nice.
Comment #6
robertDouglass CreditAttribution: robertDouglass commentedNice feature request. I agree with Steven that this is a pager issue. The best solution would be to come up with a theme pager function that gets that encapsulates this code:
And then recycles it in the search display.
Comment #7
Rowanw CreditAttribution: Rowanw commentedSubscribing.
Comment #8
Rowanw CreditAttribution: Rowanw commentedComment #9
davemybes CreditAttribution: davemybes commentedHere's a link to some code I created to get the 1-10,11-20...etc. for paged results: http://drupal.org/node/201969. Hope this helps.
Comment #10
David Lesieur CreditAttribution: David Lesieur commentedLIVE FROM THE MINNESOTA SEARCH SPRINT: Here's a first step... We'll probably want to add some default CSS for the pager information.
Comment #11
douggreen CreditAttribution: douggreen commentedHere's a second attempt, that I think is a little less invasive. It changes the name from pager_info to pager_range, calls it from the search result theming, rather than adding an argument to theme_pager, and lets the search results template print the pager wherever it wants.
Comment #12
robertDouglass CreditAttribution: robertDouglass commentedMy opinion is that theme_pager_range isn't a theme function. It has global variables, math, and a PHP control structure. I would want to see that done in a normal function and have those values passed into a real theme function that just outputs HTML.
Comment #13
David Lesieur CreditAttribution: David Lesieur commentedI've added a new pager_describe_range() function that takes the logic away from the theme function.
I've also moved the pager range above the search results, which is closer to what Google does. ;)
Comment #14
David Lesieur CreditAttribution: David Lesieur commentedRe-rolled patch with unit tests.
Comment #15
David Lesieur CreditAttribution: David Lesieur commentedComment #16
douggreen CreditAttribution: douggreen commentedIf $pager_total_items[$element] <= 1, $from and $to are not used. You might want to assign them in the conditional where they are used.
Comment #17
David Lesieur CreditAttribution: David Lesieur commentedOops, yes! Done.
Comment #18
David Lesieur CreditAttribution: David Lesieur commentedRemoved the unneeded declaration of $pager_total from pager_describe_range().
Comment #19
douggreen CreditAttribution: douggreen commentedOne more comment, FWIW,
... could be two lines shorter, but it may be clearer the way it is ... I'm not sure what the Drupal standard is here.
Comment #20
David Lesieur CreditAttribution: David Lesieur commentedI prefer your shorter version. Here's the patch!
Comment #21
douggreen CreditAttribution: douggreen commentedI applied the patch. Because there's a new theme function, I had to force rebuilding the theme and code registries. I tested the display on the first couple pages, and on the last page of a large results set. I tested the display on an empty results set. I also ran the new pager tests.
I had a hard time finding the pager test on admin/build/testing because this test is now it's own section. Should we group the test with the search tests or system tests or something else. Other than this question with where to group the pager test on admin/build/testing, I think that this is RTBC.
Comment #22
douggreen CreditAttribution: douggreen commentedI have a question above about where the test should live, but that's possibly a question for Dries, so I'm marking this RTBC.
Comment #23
Dries CreditAttribution: Dries commentedI reviewed this patch and it all looks good. The only point I have is that it doesn't look attractive visually.
I'd recommend the following: (i) we remove the title 'Search results' and (ii) make a new dynamic title "Found x items".
I think that would remove some clutter and make it look a bit more visually appealing.
Comment #24
Dries CreditAttribution: Dries commentedI'm going to mark this 'code needs work' to see if we can improve the visuals a bit. If we can't, I'll commit it as is.
Comment #25
David Lesieur CreditAttribution: David Lesieur commented@Dries: Do you see the dynamic title completely replacing the pager range description proposed in the patch?
I think a more useful title would include the search keywords, although it probably wouldn't fit very well with how the search results page is currently designed.
Comment #26
bcn CreditAttribution: bcn commentedThe first attachment (A.patch) is mostly a re-roll of the patch from #20, with one small change. I removed the title text ("Search Results"), and moved the test to the simpletest directory.
The second attachment (B.patch) is based of the first one, but I changed the formatting a bit, and added the search sting. I wasn't sure of the right way to find the search string, so this one is more of a functional prototype as opposed to a ready to commit patch...
Comment #27
bcn CreditAttribution: bcn commentedCNR to test the patch.
Comment #29
jhodgdonSigh. Yet another issue that didn't make it into Drupal 7, and needs to be postponed to Drupal 8.
Comment #30
rasumo CreditAttribution: rasumo commentedHere's a solution (far from perfect) that's implemented at the theming layer.
Code for template.php:
Code for search-results.tpl.php:
Example Result: Displaying 1 - 3 of 3 results for "INSERT TERM HERE"
NOTE: When using Apache Solr for search and you let it take over taxonomy handling, this approach may produce undesired results (working on a solution).
Comment #31
BeaPower CreditAttribution: BeaPower commentedIs this default for views in d7 or I still need to use the code in #30?
Comment #32
TechNikh CreditAttribution: TechNikh commentedthe code here worked for me in Drupal 7
http://ericlondon.com/posts/175-displaying-the-total-number-of-results-on-the-search-page-and-how-many-are-being-shown-on-the-current-page
Comment #33
jhodgdonSince 8.0.x-beta1 has been released, our policy at this point is No feature requests until 8.1.x. See #2350615: [policy, no patch] What changes can be accepted during the Drupal 8 beta phase?. Sorry, it's just too late for 8.0.x at this point, so even if we had a viable patch, the core committers would not commit it. So unless we decide this is a Task or a Bug (and I don't think it is), we'll have to delay it.
Comment #34
David Lesieur CreditAttribution: David Lesieur commentedComment #37
xeM8VfDh CreditAttribution: xeM8VfDh commentedis this still being looked into for D8? I'd like this enhancement.
Comment #50
tyler36 CreditAttribution: tyler36 at Inter Quest commentedI am going to close this because there has been no progress in 8+ years.