On opac-detail, there should be a JS popup or something like that that could display the library information(s): address, phone, email, and notes field - that could be used to specify opening hours or any other useful information.
Created attachment 7058 [details] [review] patch We have to do this in two iterations. Please have a look and comment. Once we are happy I'll extract js and css so it can be reused.
This looks like a nice enhancement, but I fear we will never agree on what should and should not be displayed in the popup... Would it be possible to add a syspref that - disables the feature if it's empty - takes a template with placeholders for the different fields that describe the libraries, so each library could format the contents of the popup to their heart's content? I'm attaching a screenshot of what the current patch looks like, to make it easier for others to add their opinion.
Created attachment 7059 [details] Screenshot of library details displayed in the OPAC
I think i agree with Magnus - not every library will want all the data from the branches table to be displayed in the OPAC. I would really like to see that as an option, a syspref with a list of fields to display perhaps and leaving it empty when you don't want it to display. With TT such things have become a lot easier to do! Is it possible to click on the mail adress? I can't see from the screenshot.
(In reply to comment #4) > Is it possible to click on the mail adress? I can't see from the screenshot. Good question! No, it's not. (At least not in FF8 on Ubuntu). The popup follows the mouse pointer, and you can't moce the mouse pointer over the popup, if you try it disappears. So turning the address into a clickable link might not make too much sense.
Please make this optional as schools generally the address of the school isn't important to our users they are just trying to locate a book at their school or can talk with the librarian about getting it. my .0001 cent opinion.
We would prefer a sytem like in news ("koha-news.pl") where you can define html code you want to display.
I think for this bug a system preference allowing to specify the fields for display would be a nice addition and could be enhanced later on. In general I like the idea to use something like the news editor/notices editor for some of the things we have in system preferences now, like most of the OPAC html preferences. - different entries for different languages - html editor to make it easier for users without html knowledge - display of available placeholders
Created attachment 7417 [details] [review] patch A different approach: * Added a new field opac_info that is mce editable and can contain HTML * Popup will display the content Please feel free to improve in styling and suggest some documentation
Created attachment 7439 [details] [review] Bug 7264 - Followup: Add a black border to the popup I think it's a bit hard to dinstinguish the popup from the background without a border, so this followup patch adds that. It should be applied on top of Srdjan's original patch.
Created attachment 7440 [details] [review] bug_7264: Added opac_info field to branches table. Rhe contents are displayed on the opac detail screen as a branch popup Signed-off-by: Magnus Enger <magnus@enger.priv.no> Applied the patch and then could enter information for each library in Administration › Libraries and Groups, with a nice MCE interface. In the OPAC the entered information is displayed in a popup, but only for those libraries that have the info, if a library lacks info it looks exactly as it used to. It's possible to mouse over the popup, so providing links in the popup will work. I think the popup needs a border to stand out more, I have created a separate, followup patch for that.
Created attachment 7441 [details] [review] bug_7264: [SIGNED-OFF] Added opac_info field to branches table. Rhe contents are displayed on the opac detail screen as a branch popup Signed-off-by: Magnus Enger <magnus@enger.priv.no> Applied the patch and then could enter information for each library in Administration › Libraries and Groups, with a nice MCE interface. In the OPAC the entered information is displayed in a popup, but only for those libraries that have the info, if a library lacks info it looks exactly as it used to. It's possible to mouse over the popup, so providing links in the popup will work. I think the popup needs a border to stand out more, I have created a separate, followup patch for that.
I have signed off on attachment 7441 [details] [review], but my followup attachment 7439 [details] [review] needs signoff (or another patch, if someone comes up with other improvements), so I leave the status at "Needs Signoff".
Created attachment 7518 [details] [review] [SIGNED-OFF] Bug 7264 - Followup: Add a black border to the popup
Applying: bug_7264: [SIGNED-OFF] Added opac_info field to branches table. /usr/share/koha/testclone/.git/rebase-apply/patch:201: trailing whitespace. * /usr/share/koha/testclone/.git/rebase-apply/patch:203: trailing whitespace. * /usr/share/koha/testclone/.git/rebase-apply/patch:205: trailing whitespace. * /usr/share/koha/testclone/.git/rebase-apply/patch:207: trailing whitespace. * error: patch failed: C4/Items.pm:1235 error: C4/Items.pm: patch does not apply error: patch failed: installer/data/mysql/updatedatabase.pl:4663 error: installer/data/mysql/updatedatabase.pl: patch does not apply warning: koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt has type 100644, expected 100755 Using index info to reconstruct a base tree... <stdin>:201: trailing whitespace. * <stdin>:203: trailing whitespace. * <stdin>:205: trailing whitespace. * <stdin>:207: trailing whitespace. * warning: 4 lines add whitespace errors. Falling back to patching base and 3-way merge... Auto-merged C4/Items.pm CONFLICT (content): Merge conflict in C4/Items.pm Auto-merged installer/data/mysql/kohastructure.sql Auto-merged installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Auto-merged koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt Failed to merge in the changes. Please rebase, fix whitespace warnings too and note file attribs. Thanks!
Created attachment 7912 [details] [review] patch
i tried to test this patch with sandbox system, it does not work, here is the result: Applying: Bug 7264 - Followup: Add a black border to the popup Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 7264 - Followup: Add a black border to the popup When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort". Bug 7264 - Display information about library on OPAC detail [SIGNED-OFF] Bug 7264 - Followup: Add a black border to the popup Apply? [yn] Patch left in /tmp/SIGNED-OFF-Bug-7264---Followup-Add-a-black-border--6LJb4S.patch
I think the problem might be that the patches are attached in the wrong order - the second attachement should be the first patch to be applied.
Yes, that is an interesting situation. I should have noted that. So the latest patch is the original patch replacement and should be applied first. The signed off patch is a follow up that needs to be applied afterwards. Now I can forsee some problems with that, so please give it a go and if it does not wotr I'll combine them into one patch.
Created attachment 8073 [details] [review] [SIGNED-OFF]Bug 7264 - Followup
I use a sandbox to test patches. In 7264 i changed the order of the patch then I started applying the patches but I have this new message: Applying: bug_7264: Branch popup Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging C4/Items.pm CONFLICT (content): Merge conflict in C4/Items.pm Auto-merging koha-tmpl/opac-tmpl/prog/en/modules/opac-detail.tt Auto-merging opac/opac-detail.pl Failed to merge in the changes. Patch failed at 0001 bug_7264: Branch popup When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort". Bug 7264 - Display information about library on OPAC detail patch Apply? [yn] Patch left in /tmp/patch-IjIQ5k.patch
Created attachment 8107 [details] [review] patch It was a wrong patch. Can I also ask you to change comment when signing off to: bug_7264: Added opac_info field to branches table. The contents are displayed on the opac detail screen as a branch popup Thanks.
Created attachment 8111 [details] [review] bug_7264: [SIGNED-OFF] Branch popup on OPAC detail.Added opac_info field to branches table. Signed-off-by: Delaye stephane <stephane.delaye@biblibre.com>
I sign this patch because it corresponds to customer requests. Since Koha Administration / branches I associated HTML (table) to a branch. Since the OPAC and data copies if I place the pointer over the title of the branch a pop up back in plain HTML (table)
Created attachment 8112 [details] [review] bug_7264: [SIGNED-OFF] Branch popup on OPAC detail.Added opac_info field to branches table. Signed-off-by: Delaye stephane <stephane.delaye@biblibre.com>
Created attachment 8113 [details] [review] bug7264-SIGNED-OFF-Branch-popup-on-OPAC-detailAdde.patch
Question: Which patches should be looked at? There is only one left now (att 8113) and rest has been obsoleted (12 atts). Please describe clearly what should be done to test this. Which field should be edited, where, etc. It is not clear to me now. Changing status to reflect the need for clarification. Thanks!
see the note 2012-03-09 09:31:46 UTC You go in Koha Administration and Libraries, branches and groups you edit the branch and in opac info you enter your html page. you use bug7264-SIGNED-OFF-Branch-popup-on-OPAC-detailAdde.patch
Thanks for responding. But where do you add this field to the database? Back to Failed QA. [Mon Mar 19 12:31:34 2012] [error] [client 192.87.126.61] [Mon Mar 19 12:31:34 2012] branches.pl: DBD::mysql::st execute failed: Unknown column 'opac_info' in 'field list' at /usr/share/koha/testclone/C4/Branch.pm line 234., referer: http://libdevelop.rijksmuseum.nl:8009/cgi-bin/koha/admin/branches.pl?op=edit&branchcode=FVM
Marcel its in the updatedatabse.pl in the patch. Did you not run that?
(In reply to comment #30) > Marcel its in the updatedatabse.pl in the patch. Did you not run that? Good point. I apparently overlooked the dbrev. Back to Signed off for QA.
QA comment: * this code is clean, no specific problem with it (perlcritic branches.pl & branches.pm [1] OK, valid&translatable templates) * it adds a jquery plugin (opac-tmpl/prog/en/lib/jquery/plugins/jquery.tools.min.js), I had a discussion about it with owen, he think this plugin will be obsoleted by jqueryUI, but until jqueryUI is not here, this plugin can be included * [1] well, Branches.pm is not OK for perlcritic after the patch, but it wasn't before too, so, this patch doesn't break of fix here.
This feature was broken by the upgrade of to jQueryUI tabs (Bug 8143).
I'm marking this fixed because the original feature was pushed and was working. A new bug was created which addresses the subsequent breakage (Bug 9572).