Add paging in opac-detail when coming from a search to be able to continue viewing the results list in the opac-detail without returning to the results list page.
Created attachment 4430 [details] [review] Proposed Patch for Paging in opac-detail when coming from a search Add paging in opac-detail when coming from a search to be able to continue viewing the previous and next register from the detail without returning to the results. The partial list pagination can be showed to increase forwarding or rewinding in the pagination. It's implemented for Zebra search and not build_grouped_results. The param busc with all the information from the search is passed encrypted in zip and base64 for security reasons and to avoid storing it in the session or submitting by POST. The Compress::Zlib perl library is needed. A list of the biblionumbers inside the OPACnumSearchResults range is passed to avoid repeating the searching everytime we see the previous or next biblio. The searching will be run again when we are going to exceed the list and a new one will be calculated from the new search.
This looks like a really, really cool new feature! Assessing the implementation of it is a little bit over my head, but I do have a couple of questions: 1. I'm not crazy about that busc-parameter in the URL of the detail view - I'm rather fond of the clean and simple URL structure of Koha... Would it be possible to store that information in a cookie/session? You say something about "avoid storing it in the session" - I'm not sure it should be avoided... ;-) 2. This is such a cool feature, I think it deserves a more prominent place than at the bottom of the screen, possibly below lots and lots of items. Could it be placed in the right hand column, as a new "box" between the existing two? (Below "more searches" and above "save record".)
(In reply to comment #2) > This looks like a really, really cool new feature! Assessing the implementation > of it is a little bit over my head, but I do have a couple of questions: > > 1. I'm not crazy about that busc-parameter in the URL of the detail view - I'm > rather fond of the clean and simple URL structure of Koha... Would it be > possible to store that information in a cookie/session? You say something about > "avoid storing it in the session" - I'm not sure it should be avoided... ;-) The reason for "avoid storing it in the session" is to be able to take the url and paste it in another place and have the same result, if you have it in a session it's impossible, and with the session you have the obligued task of cleaning all the information you've stored, and it'll have to be made from all the pages you go from the opac-detail, a lot of more coding. But I'm open to all options. I thought it was the more simple solution. > > 2. This is such a cool feature, I think it deserves a more prominent place than > at the bottom of the screen, possibly below lots and lots of items. Could it be > placed in the right hand column, as a new "box" between the existing two? > (Below "more searches" and above "save record".) Of course, I'll change it and present the new rendering. Thanks for the positive response.
(In reply to comment #3) > The reason for "avoid storing it in the session" is to be able to take the url > and paste it in another place and have the same result Hm, that doesn't seem to work for me. I copied the URL for a detail page from Firefox and pasted it into Chromium, but the next and previous links do not show up there... Here's the URL: http://localhost/cgi-bin/koha/opac-detail.pl?biblionumber=661&busc=eNo9zMEKwjAQBNBf8eRpD4ltNxskl%2F6IVJpCMDU1iVT%2F3iEFDw%2BGnWHD%2FHGP%2FTyt2%2FXlUompxbQs%0AxVene3U0b5%2B%2FmNGe50i55mebnlpZwrpFfzs2%2Fw8xlDqGewypOGZFzBp6GEDAEhvczAU6YkEWZEEv%0ADOgtsjVklIYOehja%2F5rqFJ0V%2FQNRVznG%0A
(In reply to comment #3) > The reason for "avoid storing it in the session" is to be able to take the url > and paste it in another place and have the same result That is a positive aspect of it, but I wonder if it isn't outweighed by the negative of the unfriendly URL. I think yes, but it might be a question to raise on the mailing list. I think it's much more likely that someone is going to want to share the URL of a page because of the item in question. I think the use case of wanting to share the item along with search context is less likely. (In reply to comment #4) > Hm, that doesn't seem to work for me. I copied the URL for a detail page from > Firefox and pasted it into Chromium, but the next and previous links do not > show up there... It works for me.
(In reply to comment #4) > (In reply to comment #3) > > The reason for "avoid storing it in the session" is to be able to take the url > > and paste it in another place and have the same result > > Hm, that doesn't seem to work for me. I copied the URL for a detail page from > Firefox and pasted it into Chromium, but the next and previous links do not > show up there... > > Here's the URL: > http://localhost/cgi-bin/koha/opac-detail.pl?biblionumber=661&busc=eNo9zMEKwjAQBNBf8eRpD4ltNxskl%2F6IVJpCMDU1iVT%2F3iEFDw%2BGnWHD%2FHGP%2FTyt2%2FXlUompxbQs%0AxVene3U0b5%2B%2FmNGe50i55mebnlpZwrpFfzs2%2Fw8xlDqGewypOGZFzBp6GEDAEhvczAU6YkEWZEEv%0ADOgtsjVklIYOehja%2F5rqFJ0V%2FQNRVznG%0A From what I see it's a fine url, but obviously I can't access you localhost ;) I've tested it on my server and works the copying and pasting action for Chromium, Opera, IE, Firefox, Safari. But maybe you've found a bug that I didn't expect or foresee. Can you see your logs to narrow the problem? Thanks
(In reply to comment #5) > (In reply to comment #3) > > The reason for "avoid storing it in the session" is to be able to take the url > > and paste it in another place and have the same result > > That is a positive aspect of it, but I wonder if it isn't outweighed by the > negative of the unfriendly URL. I think yes, but it might be a question to > raise on the mailing list. I think it's much more likely that someone is going > to want to share the URL of a page because of the item in question. I think the > use case of wanting to share the item along with search context is less likely. > Perfect, I'll change it as the community decides, I know it's an ugly url, so as I said before I'm open to all options. > (In reply to comment #4) > > Hm, that doesn't seem to work for me. I copied the URL for a detail page from > > Firefox and pasted it into Chromium, but the next and previous links do not > > show up there... > > It works for me.
> (In reply to comment #4) > > Hm, that doesn't seem to work for me. I copied the URL for a detail page from > > Firefox and pasted it into Chromium, but the next and previous links do not > > show up there... > > It works for me. Sorry about that, it works for me too. (I forgot to switch from Norwegian to English when testing - again...) Anyway, I do agree with Owen's comment that the benefit of this functionality probably does not outweigh the "unfriendliness" of the URL.
Created attachment 4446 [details] [review] Proposed patch for Add paging in opac-detail when coming from a search with session storage Add paging in opac-detail when coming from a search to be able to continue viewing the previous and next register from the detail without returning to the results. The partial list pagination can be showed to increase forwarding or rewinding in the pagination. It's implemented for Zebra search and not build_grouped_results. The param busc with all the information from the search is stored on the session when running opac-search and opac-detail, outside these pages the busc param is removed from the session. A list of the biblionumbers inside the OPACnumSearchResults range is passed to avoid repeating the searching everytime we see the previous or next biblio. The searching will be run again when we are going to exceed the list and a new one will be calculated from the new search.
I have tested the revised patch (attachment 4446 [details] [review]) and it works beautifully. I do have some comments, though: - the size of the text in the box on the right (Pagination: « previous next » ) should be a little larger, to match the text above and below it - I would prefer if all the added functionality was in the box on the right, and nothing was below the items - but that might just be me? This is kind of a big change to how the OPAC detail view looks, so it would be good to have some more opinions! I'll attach a couple screen shots so others can see without applying the patch.
Created attachment 4476 [details] Pagination, standard view This shows the OPAC detail view as it looks by default, with the patch applied.
Created attachment 4477 [details] Pagination, expanded view Again, this is with the patch applied, but here the "..." below the items has been clicked, to reveal the list of records that are on the same "page" as the one being displayed on the page.
(In reply to comment #10) > I have tested the revised patch (attachment 4446 [details] [review]) and it works beautifully. I > do have some comments, though: > > - the size of the text in the box on the right (Pagination: « previous next » > ) should be a little larger, to match the text above and below it You're right the font size doesn't match, I'll change the css to fix it. > > - I would prefer if all the added functionality was in the box on the right, > and nothing was below the items - but that might just be me? I've added only the simple functionality in the right box, and the same with the paging list below the items to not overload the page at the top area. But it can be modified to render it as the community decides. > > This is kind of a big change to how the OPAC detail view looks, so it would be > good to have some more opinions! I'll attach a couple screen shots so others > can see without applying the patch. Ok, good idea.
I think that the back to results button or link should be in the top left, since that's where it is on most browsers and most popular websites, that's where I always look for a back option. See WorldCat for an example (search for anything and click on any title, the link is in the top left to get back to the results. Nicole
Created attachment 4484 [details] Suggested change Here's my suggestion for the sidebar. I think in the near future we should consider changing the in-page display of the results list to use a modal dialog of some kind instead of displaying it inline in the page. However, I think the current state is fine for now in that regard. The only other thing I think we need to fix before it can be signed off on is the trigger for the display of the results list. I think the "..." is a little too obscure. How about an "Expand results" link between the previous and next links instead?
I approve of that image (since I previously said it should be elsewhere I figured I should mention that).
Created attachment 4495 [details] New look proposal for the pagination in opac-detail Hi, After looking at your proposals and consulting with our design team we have decided this new look for the pagination. The links are all on the top right, the browse results is a link to open a layer with two areas. The top one is an index list and two special links, the leftmost and rightmost are just to navigate the list. The bottom area is a list with the biblios that correspond to the index list. Everytime you move over the index list, the biblio list sorts itself to accomodate to the index list. The features hasn't changed, only the look.
Created attachment 4496 [details] Extracted first image from PDF: Pagination links
Created attachment 4497 [details] Extracted second image from PDF: Expanded results menu
I think the latest proposal looks great.
+1
I LOVE IT!! +1
Created attachment 4509 [details] [review] Proposed patch with the new look Modified patch to include the new look.
Created attachment 4512 [details] [review] Signed-off patch I can't vouch for the technical quality of the code, but the feature works beautifully. Thanks to the author for doing multiple revisions based on feedback from the Community!
Agreed, these new screenshots do look really good. Will test the functionality more in depth soon, but I wanted to provide this bit of feedback: Should this be a system-preference controlled option? Can we safely assume that all libraries will want this feature, since it's really neat, or should we err on the side of caution and make in an "opt-in" feature. If a system preference was added, should it go in OPAC preferences, or Searching preferences?
The idea was to enhance the usability and the navigation of the opac-detail page. We didn't create a new sytem preference because the patch doesn't show new information or add new content features, only links to a result already achieved. But as always the patch can be modified to give the option of activate it through a system preference if the community thinks so. I suggest the system preference to go in the Opac Preferences (Appearance) if it's decided that has to be modified.
I'd say get the feature in there and let's worry about the syspref later if anyone complains...
It's quite a big change and because it's OPAC and not staff I would like to see a system preference. Perhaps a compromise could be to turn it ON by default instead of OFF? :)
To hide this using OPACUserCSS you would add this: .nav_results { display: none; }
Good point :)
As per discussion in IRC, the need for a system preference to control this can be put off onto another ticket, since for anyone who wants to hide this, they can use the CSS provided.
(In reply to comment #31) > As per discussion in IRC, the need for a system preference to control this can > be put off onto another ticket, since for anyone who wants to hide this, they > can use the CSS provided. +1 from me for this option or not creating a syspref join me at the LASTCEBABTAASM (the "league against sysprefs that can easily be avoided, because there are already so many")
QA comment This patch does not apply anymore, there is a conflict on opac.css I think it's easy to solve, but I don't want to do something wrong, so I mark "does not apply". Salva, please rebase & fix the problem, i'll have a look ASAP, I agree it's a great feature.
Other comment : it seems this ENH has not been announced on the wiki, through a RFC. Am I right ?
Created attachment 4977 [details] [review] Rebased patch to fix conflict on opac.css
(In reply to comment #34) > Other comment : it seems this ENH has not been announced on the wiki, through a > RFC. Am I right ? You're right, I didn't think it was worth adding as is a minor enhancement, without database modification or large number of pages affected. But I'll add a RFC if you consider it necessary. BTW, looking at bugs I think it might cause a confict with bug 6120 (http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6120) as both add the back to search functionality in the same place. I see that bug 6120 doesn't add paging, only the back to results button. We'll have to evaluate whether both bugs can coexist, a merge is done or one is rejected.
(In reply to comment #36) > (In reply to comment #34) > > Other comment : it seems this ENH has not been announced on the wiki, through a > > RFC. Am I right ? > > You're right, I didn't think it was worth adding as is a minor enhancement, > without database modification or large number of pages affected. But I'll add > a RFC if you consider it necessary. well, IMO : * it's not a minor ENH ! And I just discovered the feature when QAing it. The bad news is that BibLibre just developed something very similar to this one. Lack of coordination/communication :(( (see my RM for 3.8/4.0 application on the wiki ) * in RFC, the C means 'Comment'. Once it's done, there are no real comments we can do. I would vote to continue without RFCs on the wiki. > BTW, looking at bugs I think it might cause a confict with bug 6120 > (http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6120) as both add the > back to search functionality in the same place. I see that bug 6120 doesn't add > paging, only the back to results button. We'll have to evaluate whether both > bugs can coexist, a merge is done or one is rejected. Well spotted ! i've removed the "passed QA" to bug 6120, we must discuss what we do with those conflicting developments. My personal preference would be to deal with your patch, as he does more things.
Created attachment 5114 [details] [review] Signed-off patch
There is a bug with search containing accented characters. For example, if you search 'café', the 'busc' variable used to create 'back to results' link contain: café
Shouldn't that bug be fixed before this gets a signoff?
> Shouldn't that bug be fixed before this gets a signoff? ' I'd expect a following patch from the (great) enhancement submitter. He is in situation to fix it quickly. If he can't, he could leave a note here, and I may try...
I can't seem to recreate the bug reported by Frédéric with the accented characters. I've tested the patch with Koha running on servers OpenBSD 4.9 and FreeBSD 8.2 without success. OS FreeBSD: ----------- Table sessions in mysql: mysql> show create table sessions; | sessions | CREATE TABLE `sessions` ( `id` varchar(32) NOT NULL, `a_session` text NOT NULL, UNIQUE KEY `id` (`id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8 | mysql> SELECT CCSA.character_set_name FROM information_schema.`TABLES` T, -> information_schema.`COLLATION_CHARACTER_SET_APPLICABILITY` CCSA -> WHERE CCSA.collation_name = T.table_collation AND T.table_name = 'sessions'; +--------------------+ | character_set_name | +--------------------+ | utf8 | +--------------------+ Query: Café busc variable on session: busc: "q=Caf\xC3\xA9&query=kw,wrdl,rtrn=Caf\xC3\xA9 &count=10&simple_query=Caf\xC3\xA9&listBiblios=29,30&total=2" Link back to results: Opera on PCBSD: http://192.168.1.12:8079/cgi-bin/koha/opac-search.pl?q=Caf%C3%A9 SeaMonkey on PCBSD: http://192.168.1.12:8079/cgi-bin/koha/opac-search.pl?q=Caf%C3%A9 Chromium on PCBSD: http://192.168.1.12:8079/cgi-bin/koha/opac-search.pl?q=Caf%C3%A9 When returns always shows Café with the accent fine. OS OpenBSD: ----------- mysql> show create table sessions; | sessions | CREATE TABLE `sessions` ( `id` varchar(32) NOT NULL, `a_session` text NOT NULL, UNIQUE KEY `id` (`id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8 | mysql> SELECT CCSA.character_set_name FROM information_schema.`TABLES` T, -> information_schema.`COLLATION_CHARACTER_SET_APPLICABILITY` CCSA -> WHERE CCSA.collation_name = T.table_collation AND T.table_name = 'sessions'; +--------------------+ | character_set_name | +--------------------+ | utf8 | +--------------------+ Query: Sé busc variable on session: busc: "q=S\xC3\xA9&query=kw,wrdl,rtrn=S\xC3\xA9 &count=20&simple_query=S\xC3\xA9&listBiblios=25,24,23,21,18,17,16,12,7,6,4,2&total=12" Link back to results: Opera on PCBSD: http://192.168.1.14:8079/cgi-bin/koha/opac-search.pl?q=S%C3%A9 SeaMonkey on PCBSD: http://192.168.1.14:8079/cgi-bin/koha/opac-search.pl?q=S%C3%A9 Chromium on PCBSD: http://192.168.1.14:8079/cgi-bin/koha/opac-search.pl?q=S%C3%A9 When returns always shows Sé with the accent fine. I've seen that café is the utf8 codification of Café, maybe there's a codification in some place, but in the patch there is not one. [salzaru@pcbsd /usr/home/salzaru]$ echo "Café" | iconv -t UTF-8 -f ISO-8859-1 Café [salzaru@pcbsd /usr/home/salzaru]$ perl -e 'use Encode; print (encode("utf8", "Café") . chr(10));' Café [salzaru@pcbsd /usr/home/salzaru]$ perl -e 'use Encode; print (decode("utf8", "café") . chr(10));' café Any ideas?
Thanks for your quick and detailed reply. > Query: Café > busc variable on session: busc: > "q=Caf\xC3\xA9&query=kw,wrdl,rtrn=Caf\xC3\xA9 > &count=10&simple_query=Caf\xC3\xA9&listBiblios=29,30&total=2" I haven't Caf\xC3\xA9 but Caf\xC3\x83\xC2\xA9. And I have the same sessions table charset: utf8. So something get wrong when writing the busc variable. I can reproduce the bug on whatever web browser.
I can't reproduce that bug either: <li class="back_results"><a title="Back to the results search list" href="opac-search.pl?q=café ">Back to results</a></li>
Created attachment 5988 [details] [review] Passed QA patch (rebased on current master) Patch had minor conflict in opac.css (resolved now). Marking as Passed QA
Pushed, please test
*** Bug 6120 has been marked as a duplicate of this bug. ***
*** Bug 3075 has been marked as a duplicate of this bug. ***
This is working great in production.
(In reply to comment #49) > This is working great in production. Except for bug 7394