Bug 10858 - Browse selected biblios - OPAC
Summary: Browse selected biblios - OPAC
Status: Patch doesn't apply
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low new feature (vote)
Target Milestone: ---
Assignee: Julian Maurice
QA Contact: Josef Moravec
URL:
Whiteboard:
Keywords: release-notes-needed
: 11296 (view as bug list)
Depends on:
Blocks: 11311 11296
  Show dependency treegraph
 
Reported: 2013-09-10 11:55 UTC by Jonathan Druart
Modified: 2018-06-11 14:15 UTC (History)
17 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Bot Control: ---
When did the bot last check this: 2013-09-29 00:00:00
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 10858: Browse selected biblios (20.84 KB, patch)
2013-09-10 12:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios (20.98 KB, patch)
2013-09-26 09:08 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 10858 [Follow-up] Browse selected biblios (32.49 KB, patch)
2013-09-26 13:21 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios (20.89 KB, patch)
2013-09-30 08:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858 [Follow-up] Browse selected biblios (32.42 KB, patch)
2013-09-30 08:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios (20.95 KB, patch)
2013-10-11 13:40 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 10858 [Follow-up] Browse selected biblios (32.48 KB, patch)
2013-10-11 13:40 UTC, sandboxes@biblibre.com
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios (23.51 KB, patch)
2013-12-13 19:38 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios (23.51 KB, patch)
2013-12-13 19:38 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 10858 [Follow-up] Browse selected biblios (32.72 KB, patch)
2013-12-13 19:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 10858: FIX error if no biblio is selected (1.37 KB, patch)
2013-12-18 14:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Don't use the cookie if the ft is disabled (1.07 KB, patch)
2013-12-23 12:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Update the jquery.json plugin to the lastest version (13.67 KB, patch)
2013-12-23 12:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios (23.51 KB, patch)
2014-02-19 10:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858 [Follow-up] Browse selected biblios (32.70 KB, patch)
2014-02-19 10:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: FIX error if no biblio is selected (1.37 KB, patch)
2014-02-19 10:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Don't use the cookie if the ft is disabled (1.07 KB, patch)
2014-02-19 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Update the jquery.json plugin to the lastest version (13.67 KB, patch)
2014-02-19 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios - bootstrap (27.06 KB, patch)
2014-02-19 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Remove the jQuery JSON plugin (23.24 KB, patch)
2014-02-19 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: FIX the "Browse selected" link (1.30 KB, patch)
2014-02-19 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: FIX Only one occurrence of the biblionumbers should be stored (3.55 KB, patch)
2014-02-19 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Replace "Browse results" with "Browse selected results" (2.83 KB, patch)
2014-02-19 10:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios (16.19 KB, patch)
2014-02-19 16:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858 [Follow-up] Browse selected biblios (24.58 KB, patch)
2014-02-19 16:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: FIX error if no biblio is selected (1.37 KB, patch)
2014-02-19 16:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Don't use the cookie if the ft is disabled (1.07 KB, patch)
2014-02-19 16:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios - bootstrap (21.24 KB, patch)
2014-02-19 16:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: FIX the "Browse selected" link (1.30 KB, patch)
2014-02-19 16:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: FIX Only one occurrence of the biblionumbers should be stored (3.55 KB, patch)
2014-02-19 16:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Replace "Browse results" with "Browse selected results" (2.83 KB, patch)
2014-02-19 16:37 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: fix bad merge conflict (1.24 KB, patch)
2014-02-20 15:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: follow-up Don't use the cookie if the ft is disabled (1.94 KB, patch)
2014-02-20 15:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Handle JS deactivated and cookies disabled (2.94 KB, patch)
2014-02-20 15:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios (16.29 KB, patch)
2014-02-26 14:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10858 [Follow-up] Browse selected biblios (24.67 KB, patch)
2014-02-26 14:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10858: FIX error if no biblio is selected (1.47 KB, patch)
2014-02-26 14:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10858: Don't use the cookie if the ft is disabled (1.18 KB, patch)
2014-02-26 14:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios - bootstrap (21.33 KB, patch)
2014-02-26 14:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10858: FIX the "Browse selected" link (1.39 KB, patch)
2014-02-26 14:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10858: FIX Only one occurrence of the biblionumbers should be stored (3.64 KB, patch)
2014-02-26 14:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10858: Replace "Browse results" with "Browse selected results" (2.92 KB, patch)
2014-02-26 14:48 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10858: fix bad merge conflict (1.35 KB, patch)
2014-02-26 14:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10858: follow-up Don't use the cookie if the ft is disabled (2.03 KB, patch)
2014-02-26 14:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10858: Handle JS deactivated and cookies disabled (3.02 KB, patch)
2014-02-26 14:49 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios (25.54 KB, patch)
2015-03-10 09:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios (24.24 KB, patch)
2015-04-17 11:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Define the NO_RECORD_SELECTED msg in any case (1.87 KB, patch)
2015-04-23 11:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Replace the unique jQuery function with our own (1.09 KB, patch)
2015-05-26 08:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
No way to select records with list and cart inactive - link stil shows (24.31 KB, image/png)
2015-06-06 15:24 UTC, Katrin Fischer
Details
Bug 10858: Always show checkboxes in OPAC search results (3.17 KB, patch)
2015-08-24 11:58 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios (24.14 KB, patch)
2015-08-24 12:01 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 10858: Define the NO_RECORD_SELECTED msg in any case (1.87 KB, patch)
2015-08-24 12:01 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 10858: Replace the unique jQuery function with our own (1.09 KB, patch)
2015-08-24 12:01 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 10858: Always show checkboxes in OPAC search results (3.17 KB, patch)
2015-08-24 12:02 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 10858: Show checkbox if OpacBrowseResult is set (3.38 KB, patch)
2015-11-17 08:27 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios (24.16 KB, patch)
2016-10-14 08:35 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 10858: Define the NO_RECORD_SELECTED msg in any case (1.87 KB, patch)
2016-10-14 08:35 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 10858: Replace the unique jQuery function with our own (1.09 KB, patch)
2016-10-14 08:35 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 10858: Show checkbox if OpacBrowseResult is set (3.38 KB, patch)
2016-10-14 08:36 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios (24.37 KB, patch)
2017-09-12 07:13 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 10858: Define the NO_RECORD_SELECTED msg in any case (1.87 KB, patch)
2017-09-12 07:13 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 10858: Replace the unique jQuery function with our own (1.10 KB, patch)
2017-09-12 07:13 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 10858: Show checkbox if OpacBrowseResult is set (3.39 KB, patch)
2017-09-12 07:13 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios (24.32 KB, patch)
2017-09-12 14:12 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios (24.38 KB, patch)
2017-09-12 14:21 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 10858: Define the NO_RECORD_SELECTED msg in any case (1.92 KB, patch)
2017-09-12 14:21 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 10858: Replace the unique jQuery function with our own (1.15 KB, patch)
2017-09-12 14:21 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 10858: Show checkbox if OpacBrowseResult is set (3.44 KB, patch)
2017-09-12 14:22 UTC, Nicolas Legrand
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios (24.89 KB, patch)
2017-10-11 19:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Define the NO_RECORD_SELECTED msg in any case (1.93 KB, patch)
2017-10-11 19:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Replace the unique jQuery function with our own (1.16 KB, patch)
2017-10-11 19:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Show checkbox if OpacBrowseResult is set (3.45 KB, patch)
2017-10-11 19:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Add 2 missing nav class (1.72 KB, patch)
2017-10-11 20:00 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 10858: Browse selected biblios (24.54 KB, patch)
2018-02-05 09:34 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 10858: Define the NO_RECORD_SELECTED msg in any case (1.98 KB, patch)
2018-02-05 09:34 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 10858: Replace the unique jQuery function with our own (1.21 KB, patch)
2018-02-05 09:35 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 10858: Show checkbox if OpacBrowseResult is set (3.50 KB, patch)
2018-02-05 09:35 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 10858: Add 2 missing nav class (1.77 KB, patch)
2018-02-05 09:35 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 10858: (QA follow-up) Fix translatibility (2.08 KB, patch)
2018-02-05 09:35 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2013-09-10 11:55:17 UTC
After launching a search, lots of people would like to browse some results from several pages.
Comment 1 Jonathan Druart 2013-09-10 12:31:06 UTC Comment hidden (obsolete)
Comment 2 sandboxes@biblibre.com 2013-09-26 09:08:11 UTC
Patch tested with a sandbox, by Cédric Vita <cedric.vita@dracenie.com>
Comment 3 sandboxes@biblibre.com 2013-09-26 09:08:29 UTC Comment hidden (obsolete)
Comment 4 Owen Leonard 2013-09-26 13:21:39 UTC Comment hidden (obsolete)
Comment 5 I'm just a bot 2013-09-29 06:18:28 UTC
Applying: Bug 10858 [Follow-up] Browse selected biblios
Using index info to reconstruct a base tree...
M	koha-tmpl/opac-tmpl/prog/en/css/opac.css
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/opac-tmpl/prog/en/css/opac.css
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/prog/en/css/opac.css
Patch failed at 0001 Bug 10858 [Follow-up] Browse selected biblios
The copy of the patch that failed is found in:
   /home/christopher/git/koha/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Comment 6 Jonathan Druart 2013-09-30 08:16:59 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2013-09-30 08:17:08 UTC Comment hidden (obsolete)
Comment 8 sandboxes@biblibre.com 2013-10-11 13:40:13 UTC
Patch tested with a sandbox, by Koha Team Amu <koha.aixmarseille@gmail.com>
Comment 9 sandboxes@biblibre.com 2013-10-11 13:40:39 UTC Comment hidden (obsolete)
Comment 10 sandboxes@biblibre.com 2013-10-11 13:40:43 UTC Comment hidden (obsolete)
Comment 11 Koha Team AMU 2013-10-11 13:43:04 UTC
Potential development : to have a list of results after the click on the "Browse selected biblios"
Comment 12 Kyle M Hall 2013-11-22 19:13:26 UTC
Looks good for prog and ccsr opac themes. However, the feature needs to be updated to work with the new bootstrap theme as well.
Comment 13 Jonathan Druart 2013-11-25 11:45:59 UTC
(In reply to Kyle M Hall from comment #12)

I will open a new report for the bootstrap theme.

Back to Signed off
Comment 14 Kyle M Hall 2013-12-13 19:38:08 UTC Comment hidden (obsolete)
Comment 15 Kyle M Hall 2013-12-13 19:38:43 UTC Comment hidden (obsolete)
Comment 16 Kyle M Hall 2013-12-13 19:39:26 UTC Comment hidden (obsolete)
Comment 17 Galen Charlton 2013-12-13 20:40:15 UTC
General question about this proposed feature: what's wrong with using the cart for this?
Comment 18 Jonathan Druart 2013-12-18 12:52:27 UTC
(In reply to Galen Charlton from comment #17)
> General question about this proposed feature: what's wrong with using the
> cart for this?

The idea is to navigate between selected results, I don't see how to do that with the cart.
When I open the basket, I can see a biblio list but the detail page is opened in a new window.
Even if I improve the behavior, on a ux point of view I think it is not friendly to have to put biblios in the cart for browsing them.
Moreover the same enhancement is developed for authorities (bug 11311), which can not be put in the cart.
Comment 19 Jonathan Druart 2013-12-18 14:31:47 UTC Comment hidden (obsolete)
Comment 20 Katrin Fischer 2013-12-21 00:24:12 UTC
Hm, I just took a look at the feature and I think I found a problem:

1) The feature should not be active, when OpacBrowseResults is turned off. Maybe there should even be a separate pref to control this independently from OpacBrowseResults. People are worried about the use of cookies in some contexts and it might be nice to limit the use of them to a mininum.

2) I wonder if the plugin used needs to be added to the about.pl page - not sure there. It also states alpha, but it does look like there is a stable version available now: https://code.google.com/p/jquery-json/

3) While this feature is a bit fun to use, I wonder a bit about the use cases. Maybe the sponsoring library/libraires could share the idea behind the feature?

4) Currently the feature seems to do less than the cart or the list functionality offer, as there are no 'bulk' actions available, so I can understand what Galen means with his comment. 

5) Also it changes the behaviour of an existing feature, which might be a bit unexpected to users. Not sure what we could do here.

6) When switching between different records you always have to reopen the "Browse results" area and this hides part of the possible actions on the page. This way you can't switch really fast between records and have to find your place in the list again.

I think the first point is a 'Failed QA'.
Comment 21 Jonathan Druart 2013-12-23 12:02:36 UTC
(In reply to Katrin Fischer from comment #20)
> Hm, I just took a look at the feature and I think I found a problem:
> 
> 1) The feature should not be active, when OpacBrowseResults is turned off.
> Maybe there should even be a separate pref to control this independently
> from OpacBrowseResults. People are worried about the use of cookies in some
> contexts and it might be nice to limit the use of them to a mininum.

> 2) I wonder if the plugin used needs to be added to the about.pl page - not
> sure there. It also states alpha, but it does look like there is a stable
> version available now: https://code.google.com/p/jquery-json/

I will provide a followup for these 2 points.

> 3) While this feature is a bit fun to use, I wonder a bit about the use
> cases. Maybe the sponsoring library/libraires could share the idea behind
> the feature?

I will ask the customer for an answer.

> 4) Currently the feature seems to do less than the cart or the list
> functionality offer, as there are no 'bulk' actions available, so I can
> understand what Galen means with his comment. 

The goal is to browse results, not to export them or any other actions.

> 5) Also it changes the behaviour of an existing feature, which might be a
> bit unexpected to users. Not sure what we could do here.

I am not sure that it changes the existing feature. If you click on a detail link (from the result list), you browse all results. If you want to browse selected biblios, you have to click on the new link "browse selected biblios" above the result list. It is 2 different features (but using the same browser UI).

> 6) When switching between different records you always have to reopen the
> "Browse results" area and this hides part of the possible actions on the
> page. This way you can't switch really fast between records and have to find
> your place in the list again.

Yes, it is the current behavior. I think this feature should be used using the next/previous links.
Comment 22 Jonathan Druart 2013-12-23 12:03:24 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2013-12-23 12:03:32 UTC Comment hidden (obsolete)
Comment 24 Jonathan Druart 2013-12-23 12:04:30 UTC
(In reply to Katrin Fischer from comment #20)
> 2) I wonder if the plugin used needs to be added to the about.pl page - not
> sure there.

I am not sure neither.
Comment 25 Yves Tomic 2014-01-13 13:14:35 UTC
The purpose of this patch is to give the possibility to users to select some results while browsing the results pages without going to the detail page immediately. After having browsed the results, the users can display the selected results and navigate through them.
Comment 26 Marcel de Rooy 2014-02-18 18:03:07 UTC
Some prelimary QA comments/questions:

From a functional perspective: I would like to see this feature in Koha. In earlier comments some mentioned the cart. But this is an interesting addition! Note also that I more than once heard the question: Why does Koha not remember the items I selected on another search result page? So even without browsing, you make some people happy.
If you browse the selection, it would imo be good to show that you are doing so (Browse selected results vs. Browse results).
If we use the js plugin, it would be good to acknowledge that on About, separate from the question if this specific MIT license needs that by law or so.
Also I do not think that remembering a few biblios in a cookie, would be a big legal problem. We c/should clear the contents every time we start a new search. And if we are not allowed to create it, well we do not.. (We have some laws in the Netherlands too about confirming the use of cookies, but in the meantime things seem to go already in a less 

More technically:
There is a small merge conflict in koha-tmpl/opac-tmpl/prog/en/css/opac.css.
Note that I would like to see the bootstrap patch here also instead of on another report. Bootstrap is now leading and should be pushed first. (Is there any trouble there with less files?) 
From a "minimalistic approach" I do wonder if we really need a new plugin (156 lines) for storing just a few checkboxes (no offense). This is more a personal opinion too and would not be blocking. But could you on the other hand make more clear why we definitely need this plugin and cannot do it with what we have now [ within say 156 lines of code :) ] ?

Waiting for some feedback. Thanks.
Comment 27 Marcel de Rooy 2014-02-18 18:04:43 UTC
in a less -> in a less restrictive way
Comment 28 Jonathan Druart 2014-02-19 10:38:54 UTC
(In reply to M. de Rooy from comment #26)
> Some prelimary QA comments/questions:

Thank you Marcel for having a look at this one.

> If you browse the selection, it would imo be good to show that you are doing
> so (Browse selected results vs. Browse results).

Will be fixed.

> If we use the js plugin, it would be good to acknowledge that on About,
> separate from the question if this specific MIT license needs that by law or
> so.

In fact the plugin is useless, I will remove it.

> Also I do not think that remembering a few biblios in a cookie, would be a
> big legal problem. We c/should clear the contents every time we start a new
> search. And if we are not allowed to create it, well we do not..

How do we know if we are allowed or not?

> More technically:
> There is a small merge conflict in koha-tmpl/opac-tmpl/prog/en/css/opac.css.

Will be fixed.

> Note that I would like to see the bootstrap patch here also instead of on
> another report. Bootstrap is now leading and should be pushed first. (Is
> there any trouble there with less files?)

The first patches were signed off, so I prefer to create another report for the bootstrap theme.
I will add the patch from bug 11296 to this one.
Comment 29 Jonathan Druart 2014-02-19 10:40:45 UTC Comment hidden (obsolete)
Comment 30 Jonathan Druart 2014-02-19 10:40:52 UTC Comment hidden (obsolete)
Comment 31 Jonathan Druart 2014-02-19 10:40:59 UTC Comment hidden (obsolete)
Comment 32 Jonathan Druart 2014-02-19 10:41:05 UTC Comment hidden (obsolete)
Comment 33 Jonathan Druart 2014-02-19 10:41:11 UTC Comment hidden (obsolete)
Comment 34 Jonathan Druart 2014-02-19 10:41:18 UTC Comment hidden (obsolete)
Comment 35 Jonathan Druart 2014-02-19 10:41:24 UTC Comment hidden (obsolete)
Comment 36 Jonathan Druart 2014-02-19 10:41:31 UTC Comment hidden (obsolete)
Comment 37 Jonathan Druart 2014-02-19 10:41:37 UTC Comment hidden (obsolete)
Comment 38 Jonathan Druart 2014-02-19 10:41:44 UTC Comment hidden (obsolete)
Comment 39 Jonathan Druart 2014-02-19 10:48:36 UTC
*** Bug 11296 has been marked as a duplicate of this bug. ***
Comment 40 Marcel de Rooy 2014-02-19 15:18:46 UTC
Jonathan:
Thanks for your adjustments and your patience in looking at what follows :)

I would strongly prefer to slightly adjust a few patches here.
We now add the plugin, update it and then remove it in this series..
I would not recommend to push that line of action if we can just remove it from the first patch. I have no objection to leaving the original signoff on that one when you enter your additional signoff commenting on the amend.
After that I will add still another signoff.
Makes that sense?

Changing status to reflect this.. Please change back to SO after that.
Comment 41 Jonathan Druart 2014-02-19 16:36:24 UTC Comment hidden (obsolete)
Comment 42 Jonathan Druart 2014-02-19 16:36:30 UTC Comment hidden (obsolete)
Comment 43 Jonathan Druart 2014-02-19 16:36:36 UTC Comment hidden (obsolete)
Comment 44 Jonathan Druart 2014-02-19 16:36:42 UTC Comment hidden (obsolete)
Comment 45 Jonathan Druart 2014-02-19 16:36:48 UTC Comment hidden (obsolete)
Comment 46 Jonathan Druart 2014-02-19 16:36:54 UTC Comment hidden (obsolete)
Comment 47 Jonathan Druart 2014-02-19 16:37:00 UTC Comment hidden (obsolete)
Comment 48 Jonathan Druart 2014-02-19 16:37:06 UTC Comment hidden (obsolete)
Comment 49 Jonathan Druart 2014-02-19 16:41:00 UTC
(In reply to M. de Rooy from comment #40)
> I would strongly prefer to slightly adjust a few patches here.
> We now add the plugin, update it and then remove it in this series..
> I would not recommend to push that line of action if we can just remove it
> from the first patch. I have no objection to leaving the original signoff on
> that one when you enter your additional signoff commenting on the amend.
> After that I will add still another signoff.
> Makes that sense?

Absolutely, it was a "morning laziness" :)
I completely removed all references to this plugin in the last set of patches.
Comment 50 Marcel de Rooy 2014-02-20 14:22:16 UTC
Some comments after experimenting the feature further on bootstrap/prog. Should we still put time in ccsr?

1. If you disable OpacBrowseResults, please remove the link.
2. In bootstrap clicking Browse selected on detail page does not respond (prog does)
3. This is a weird one but just mentioning: Disable cookies in your browser. Check two biblios on the same page. Click on Browse selected. No item was selected. Heh. If you cannot save the cookie, remove link too? Note that by checking the cookie again in setContextBiblioNumbers, you know if the first call succeeded or not.

Waiting for some feedback. After that I will still have to look at the code somewhat more closely..
Comment 51 Jonathan Druart 2014-02-20 15:51:21 UTC Comment hidden (obsolete)
Comment 52 Jonathan Druart 2014-02-20 15:51:29 UTC Comment hidden (obsolete)
Comment 53 Jonathan Druart 2014-02-20 15:51:35 UTC Comment hidden (obsolete)
Comment 54 Jonathan Druart 2014-02-20 15:53:45 UTC
(In reply to M. de Rooy from comment #50)

> 1. If you disable OpacBrowseResults, please remove the link.

Fixed

> 2. In bootstrap clicking Browse selected on detail page does not respond
> (prog does)

Cause by a bad merge conflict reintroduced by last set of patches. Fixed!

> 3. This is a weird one but just mentioning: Disable cookies in your browser.
> Check two biblios on the same page. Click on Browse selected. No item was
> selected. Heh. If you cannot save the cookie, remove link too? Note that by
> checking the cookie again in setContextBiblioNumbers, you know if the first
> call succeeded or not.

Fixed too with the last patch. There were some weird behaviors if javascript was deactivated or cookies disabled.
Comment 55 Marcel de Rooy 2014-02-26 14:48:00 UTC Comment hidden (obsolete)
Comment 56 Marcel de Rooy 2014-02-26 14:48:12 UTC Comment hidden (obsolete)
Comment 57 Marcel de Rooy 2014-02-26 14:48:20 UTC Comment hidden (obsolete)
Comment 58 Marcel de Rooy 2014-02-26 14:48:27 UTC Comment hidden (obsolete)
Comment 59 Marcel de Rooy 2014-02-26 14:48:34 UTC Comment hidden (obsolete)
Comment 60 Marcel de Rooy 2014-02-26 14:48:40 UTC Comment hidden (obsolete)
Comment 61 Marcel de Rooy 2014-02-26 14:48:47 UTC Comment hidden (obsolete)
Comment 62 Marcel de Rooy 2014-02-26 14:48:54 UTC Comment hidden (obsolete)
Comment 63 Marcel de Rooy 2014-02-26 14:49:00 UTC Comment hidden (obsolete)
Comment 64 Marcel de Rooy 2014-02-26 14:49:11 UTC Comment hidden (obsolete)
Comment 65 Marcel de Rooy 2014-02-26 14:49:19 UTC Comment hidden (obsolete)
Comment 66 Marcel de Rooy 2014-02-26 14:53:24 UTC
QA Comment:
Nice work including QA changes. No complaints from koha-qa. 

Tested prog and bootstrap. Disabled cookies too for a short while :) 

Still found small debatable issue: 
First I selected one record on page 1, page 2 and page 3. After that I selected all records on page 1. 
It confused me at first to discover that the Browse results on detail are not in the order of the search results, but apparently in the order of being added to the list. 
No blocker for me, but we could argue about the order of the list. 

One of the commit messages promised this: 
  In addition to prog and ccsr templates, this patch allows to add 
  selected biblios (from multiple pages) to the cart or a list. 
  To test: 
  - check some biblios from several pages 
  - add them to the cart 
  - add them to a list 
Unfortunately, when I added items from two pages, only the item of the current page was added. 
Again (with this being my last QA before temporary cutoff date) no blocker for me :) 
A followup would be pretty on another report though.  

Saw some warnings in the log: 
opac-detail.pl: Use of uninitialized value in concatenation (.) or string at /usr/share/koha/testclone/opac/opac-detail.pl line 394 
opac-detail.pl: Use of uninitialized value in concatenation (.) or string at /usr/share/koha/testclone/opac/opac-detail.pl line 400. 
But I have the impression that your patches did not touch these lines.. 

Passed QA
Comment 67 Jonathan Druart 2014-02-26 15:44:29 UTC
(In reply to M. de Rooy from comment #66)
> One of the commit messages promised this: 
>   In addition to prog and ccsr templates, this patch allows to add 
>   selected biblios (from multiple pages) to the cart or a list. 
>   To test: 
>   - check some biblios from several pages 
>   - add them to the cart 
>   - add them to a list 
> Unfortunately, when I added items from two pages, only the item of the
> current page was added. 

I just retested right now and it works using the bootstrap theme.
Comment 68 Galen Charlton 2014-05-05 21:49:02 UTC
This set up rather a lot of infrastructure for a feature whose workflow purpose can be met by the cart, and I remain dubious.  Consequently, I'm punting this to the next RM.
Comment 69 Jonathan Druart 2014-07-16 13:38:40 UTC
I removed the "in discussion" status. Sent to the RM queue.
Comment 70 Jonathan Druart 2015-01-23 15:08:24 UTC
Dear RM, what about this feature? :)
Comment 71 Jonathan Druart 2015-03-10 09:31:01 UTC Comment hidden (obsolete)
Comment 72 Jonathan Druart 2015-03-10 09:35:11 UTC
Ok so, back on this one.
It did not apply at all and had ccsr+prog themes.
This new patch is just a squash patch with only the bootstrap theme changes.
Test plan is the same.

QA Note:(In reply to M. de Rooy from comment #26)
> If you browse the selection, it would imo be good to show that you are doing
> so (Browse selected results vs. Browse results).

I removed this change, it did not work.
Actually I don't see an easy way to do that and consider it as a minor and should not be a blocker.

Switch back in the QA queue to give confidence to the RM.
Comment 73 Marcel de Rooy 2015-04-17 10:17:31 UTC
CONFLICT (add/add): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/lib/jquery/plugins/jquery.cookie.min.js
Auto-merging koha-tmpl/opac-tmpl/bootstrap/js/basket.js
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt
Comment 74 Jonathan Druart 2015-04-17 11:07:26 UTC Comment hidden (obsolete)
Comment 75 Katrin Fischer 2015-04-23 09:49:32 UTC
1) Shoudl this be included on the about.pl page?
+// Extends jQuery API
+// http://www.wskidmore.com/downloads/jquery-uniqueArray.min.js

2) I think there are display problems, when both the cart and the lists are turned off:

For example, this is only available with the prefs ON:
 var MSG_NO_RECORD_SELECTED = _("No item was selected");

Browse selected record still shows up, but I see only one checkbox on my third record in the list? (confusing)

I am not logged in and tried in 2 different browsers.
Comment 76 Jonathan Druart 2015-04-23 11:48:13 UTC Comment hidden (obsolete)
Comment 77 Jonathan Druart 2015-04-23 11:56:31 UTC
(In reply to Katrin Fischer from comment #75)
> 1) Shoudl this be included on the about.pl page?
> +// Extends jQuery API
> +// http://www.wskidmore.com/downloads/jquery-uniqueArray.min.js

The domain is down and I don't know at all where I found this piece of code.
It was certainly under the MIT license.

> 2) I think there are display problems, when both the cart and the lists are
> turned off:
> 
> For example, this is only available with the prefs ON:
>  var MSG_NO_RECORD_SELECTED = _("No item was selected");

Ok fixed.

> Browse selected record still shows up, but I see only one checkbox on my
> third record in the list? (confusing)
> 
> I am not logged in and tried in 2 different browsers.

I don't get it, I have never seen that.
Please provide more info to recreate the issue.
Comment 78 Katrin Fischer 2015-05-25 07:11:55 UTC
> > 1) Shoudl this be included on the about.pl page?
> > +// Extends jQuery API
> > +// http://www.wskidmore.com/downloads/jquery-uniqueArray.min.js
> 
> The domain is down and I don't know at all where I found this piece of code.
> It was certainly under the MIT license.

I am not very comfortable with the dependency as it doesn't include a lisence statement right now and there seems no way to trace it back to an author. Could we replace this with our own code or some other plugin?

Some of the other problems went away after making sure my browser cache was cleared. But I still see a problem when both cart and list are turned off: the text "browse selected records" still shows up, but there are no checkboxes to select a record.
Comment 79 Jonathan Druart 2015-05-26 08:53:08 UTC Comment hidden (obsolete)
Comment 80 Jonathan Druart 2015-05-26 08:55:01 UTC
(In reply to Katrin Fischer from comment #78)
> Some of the other problems went away after making sure my browser cache was
> cleared. But I still see a problem when both cart and list are turned off:
> the text "browse selected records" still shows up, but there are no
> checkboxes to select a record.

This is still valid.
Comment 81 Katrin Fischer 2015-05-31 12:06:45 UTC
Hi Jonathan, thx for the follow-up replacing the jQuery! Are we waiting for another follow-up  here?
Comment 82 Jonathan Druart 2015-06-03 08:44:39 UTC
No sorry, back to Needs QA.
Comment 83 Katrin Fischer 2015-06-06 15:24:09 UTC
Created attachment 39938 [details]
No way to select records with list and cart inactive - link stil shows

Hi Jonathan, I think the link should disappear when neither cart nor list are available - or we need to keep showing the checkboxes for selecting records in the result list.
Comment 84 Julian Maurice 2015-08-24 11:58:54 UTC Comment hidden (obsolete)
Comment 85 Julian Maurice 2015-08-24 12:01:38 UTC
Created attachment 41839 [details] [review]
Bug 10858: Browse selected biblios

Rebased on master
Comment 86 Julian Maurice 2015-08-24 12:01:49 UTC
Created attachment 41840 [details] [review]
Bug 10858: Define the NO_RECORD_SELECTED msg in any case

Rebased on master
Comment 87 Julian Maurice 2015-08-24 12:01:59 UTC
Created attachment 41841 [details] [review]
Bug 10858: Replace the unique jQuery function with our own

Rebased on master
Comment 88 Julian Maurice 2015-08-24 12:02:10 UTC Comment hidden (obsolete)
Comment 89 Katrin Fischer 2015-10-20 22:44:05 UTC
Hi Julian,

I think the last patch makes a wrong assumption - the browse feature is useless if OpacBrowseResults is turned off (which some of our libraries actually have). So I think removing the checks for the checkboxes should be undone and a check for this pref added to the code - for the checkboxes and also for the link at the top of the results.

What do you think?
Comment 90 Julian Maurice 2015-11-17 08:07:45 UTC
Hi Katrin,

I made this assumption by looking at the code, but the code is wrong, and you are right ;)
I will submit a followup, replacing the last patch.
Comment 91 Julian Maurice 2015-11-17 08:27:25 UTC Comment hidden (obsolete)
Comment 92 Frédéric Demians 2015-12-10 18:00:58 UTC
On last patch, I don't understand that, the last test:

[% IF
   ( Koha.Preference('opacbookbag') == 1 )
|| ( Koha.Preference('virtualshelves') == 1 )
|| ( Koha.Preference('OpacBrowseResults') == 1 )
|| ( Koha.Preference('RequestOnOpac') == 1 && (not SEARCH_RESULT.norequests) && Koha.Preference('opacuserlogin') )
%]

I think that it's rather:

|| ( Koha.Preference('RequestOnOpac') == 1 && (not SEARCH_RESULT.norequests) && (not Koha.Preference('opacuserlogin')) )

But I'm not sure...

And When everything is disabled, no checkboxes at all, links 'check all' / 'clear all' are still displayed. This bug is also in master, but still...
Comment 93 Katrin Fischer 2015-12-13 21:39:02 UTC
hm, the last line seems correct to me: if holds and login are allowed...
Comment 94 Julian Maurice 2016-10-14 08:35:07 UTC
Created attachment 56479 [details] [review]
Bug 10858: Browse selected biblios

After launching a search, some people would like to browse some results
from several pages.

This patch adds, at the OPAC, a link "Browse selected biblios >>" when
at least one biblio is checked. You can choose biblios from several
page, not the current one. In this way, checkboxes already checked
become again checked when you come back on this results page.
After clicking the link, the "Browse results" block is filled with
selected biblios, not (like as the current behavior) with the classic
results.

Test plan:
- Launch a search at the OPAC
- Check some biblios
- Go on another page
- Check some biblios
- Come back to the first page and check that already checked biblio is
  automatically checked.
- Click on the "Browse selected biblios >>" link
- Check that you are able to browse results you had checked.

You can also:
- check some biblios from several pages
- add them to the cart
- add them to a list

Signed-off-by: Cedric Vita <cedric.vita@dracenie.com>

Signed-off-by: Koha Team Amu <koha.aixmarseille@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 95 Julian Maurice 2016-10-14 08:35:20 UTC
Created attachment 56480 [details] [review]
Bug 10858: Define the NO_RECORD_SELECTED msg in any case

And not only when cart or list are defined.
Comment 96 Julian Maurice 2016-10-14 08:35:33 UTC Comment hidden (obsolete)
Comment 97 Julian Maurice 2016-10-14 08:36:02 UTC Comment hidden (obsolete)
Comment 98 Owen Leonard 2016-10-17 16:20:43 UTC
(In reply to Marcel de Rooy from comment #26)

> If you browse the selection, it would imo be good to show that you are doing
> so (Browse selected results vs. Browse results).

I like this suggestion too, but I think it could be handled in a follow-up.

It looks to me like the contents of common.js could be put into global.js and we could avoid having a separate file. Do you think there is a reason not to?
Comment 99 Jonathan Druart 2016-10-18 07:30:00 UTC
(In reply to Owen Leonard from comment #98)
> (In reply to Marcel de Rooy from comment #26)
> 
> > If you browse the selection, it would imo be good to show that you are doing
> > so (Browse selected results vs. Browse results).
> 
> I like this suggestion too, but I think it could be handled in a follow-up.
> 
> It looks to me like the contents of common.js could be put into global.js
> and we could avoid having a separate file. Do you think there is a reason
> not to?

These patches have been submitted 3 years ago, and have been rewritten 3 times already. If we want additional behaviours they should go to another bug report.
The commons.js content could be moved to global.js but note that bug 11311 is adding more content to this file. Maybe we could move it to something less vague than 'commons' but keep it apart from global.
Comment 100 Owen Leonard 2016-10-18 13:46:07 UTC
(In reply to Jonathan Druart from comment #99)
> These patches have been submitted 3 years ago, and have been rewritten 3
> times already.

Okay that's fair. My comments are not about issues which should block this.

One thing I am concerned about is the behavior of browsing when you have multiple tabs open. Browsing selected records seems to get confused if you perform a search in another tab, with the list of selected records getting replaced by the list of searched records.
Comment 101 Jonathan Druart 2016-10-18 14:11:47 UTC
Yes indeed, launching several searches in different tabs will give wrong results.
I have no idea how we could fix that.
Comment 102 Eric 2017-03-23 14:19:21 UTC
Doesn't apply in the sandboxes. Could you rebase it? Thank you
Comment 103 Julian Maurice 2017-09-12 07:13:13 UTC Comment hidden (obsolete)
Comment 104 Julian Maurice 2017-09-12 07:13:18 UTC Comment hidden (obsolete)
Comment 105 Julian Maurice 2017-09-12 07:13:23 UTC Comment hidden (obsolete)
Comment 106 Julian Maurice 2017-09-12 07:13:28 UTC Comment hidden (obsolete)
Comment 107 Julian Maurice 2017-09-12 07:14:08 UTC
Patches rebased on master
Comment 108 Nicolas Legrand 2017-09-12 14:12:40 UTC Comment hidden (obsolete)
Comment 109 Nicolas Legrand 2017-09-12 14:21:30 UTC Comment hidden (obsolete)
Comment 110 Nicolas Legrand 2017-09-12 14:21:40 UTC Comment hidden (obsolete)
Comment 111 Nicolas Legrand 2017-09-12 14:21:53 UTC Comment hidden (obsolete)
Comment 112 Nicolas Legrand 2017-09-12 14:22:38 UTC Comment hidden (obsolete)
Comment 113 Jonathan Druart 2017-10-11 19:55:11 UTC
Created attachment 67974 [details] [review]
Bug 10858: Browse selected biblios

After launching a search, some people would like to browse some results
from several pages.

This patch adds, at the OPAC, a link "Browse selected biblios >>" when
at least one biblio is checked. You can choose biblios from several
page, not the current one. In this way, checkboxes already checked
become again checked when you come back on this results page.
After clicking the link, the "Browse results" block is filled with
selected biblios, not (like as the current behavior) with the classic
results.

Test plan:
- Launch a search at the OPAC
- Check some biblios
- Go on another page
- Check some biblios
- Come back to the first page and check that already checked biblio is
  automatically checked.
- Click on the "Browse selected biblios >>" link
- Check that you are able to browse results you had checked.

You can also:
- check some biblios from several pages
- add them to the cart
- add them to a list

Signed-off-by: Cedric Vita <cedric.vita@dracenie.com>

Signed-off-by: Koha Team Amu <koha.aixmarseille@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Comment 114 Jonathan Druart 2017-10-11 19:55:23 UTC
Created attachment 67975 [details] [review]
Bug 10858: Define the NO_RECORD_SELECTED msg in any case

And not only when cart or list are defined.

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Comment 115 Jonathan Druart 2017-10-11 19:55:30 UTC
Created attachment 67976 [details] [review]
Bug 10858: Replace the unique jQuery function with our own

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Comment 116 Jonathan Druart 2017-10-11 19:55:36 UTC
Created attachment 67977 [details] [review]
Bug 10858: Show checkbox if OpacBrowseResult is set

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>
Comment 117 Jonathan Druart 2017-10-11 20:00:01 UTC
Created attachment 67978 [details] [review]
Bug 10858: Add 2 missing nav class
Comment 118 Josef Moravec 2018-02-05 09:34:49 UTC
Created attachment 71218 [details] [review]
Bug 10858: Browse selected biblios

After launching a search, some people would like to browse some results
from several pages.

This patch adds, at the OPAC, a link "Browse selected biblios >>" when
at least one biblio is checked. You can choose biblios from several
page, not the current one. In this way, checkboxes already checked
become again checked when you come back on this results page.
After clicking the link, the "Browse results" block is filled with
selected biblios, not (like as the current behavior) with the classic
results.

Test plan:
- Launch a search at the OPAC
- Check some biblios
- Go on another page
- Check some biblios
- Come back to the first page and check that already checked biblio is
  automatically checked.
- Click on the "Browse selected biblios >>" link
- Check that you are able to browse results you had checked.

You can also:
- check some biblios from several pages
- add them to the cart
- add them to a list

Signed-off-by: Cedric Vita <cedric.vita@dracenie.com>

Signed-off-by: Koha Team Amu <koha.aixmarseille@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 119 Josef Moravec 2018-02-05 09:34:55 UTC
Created attachment 71219 [details] [review]
Bug 10858: Define the NO_RECORD_SELECTED msg in any case

And not only when cart or list are defined.

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 120 Josef Moravec 2018-02-05 09:35:00 UTC
Created attachment 71220 [details] [review]
Bug 10858: Replace the unique jQuery function with our own

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 121 Josef Moravec 2018-02-05 09:35:04 UTC
Created attachment 71221 [details] [review]
Bug 10858: Show checkbox if OpacBrowseResult is set

Signed-off-by: Nicolas Legrand <nicolas.legrand@bulac.fr>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 122 Josef Moravec 2018-02-05 09:35:09 UTC
Created attachment 71222 [details] [review]
Bug 10858: Add 2 missing nav class

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 123 Josef Moravec 2018-02-05 09:35:15 UTC
Created attachment 71223 [details] [review]
Bug 10858: (QA follow-up) Fix translatibility

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 124 Jonathan Druart 2018-02-07 17:16:14 UTC
As bug 19290 adds a new pref to turn the feature on/off, should not we do the same for the OPAC?
Comment 125 Séverine Queune 2018-04-20 17:15:58 UTC
(In reply to Jonathan Druart from comment #124)
> As bug 19290 adds a new pref to turn the feature on/off, should not we do
> the same for the OPAC?

To me, that would be more logical.
Comment 126 Nicolas Legrand 2018-06-11 14:15:55 UTC
failed with new JS name scheme.