Created attachment 41441 [details] cite option & pop up It would be nice if we could add a 'cite' option to the menu on the right of the opac detail pages like in some other discovery interfaces - see attached.
Created attachment 42971 [details] [review] Bug 14670: Add Cite-option to detail page in OPAC Adds a "cite" option in the tools menu in OPAC detail page for easy citations. Test plan: 1. Apply this patch 2. Got to OPAC detail page 3. In the tools menu to the right, click the new "Cite" link 4. A modal should appear with four different citation styles (APA, Chicago, Harvard, MLA) Sponsored-by: Regionbibliotek Halland / County library of Halland
Created attachment 43508 [details] [review] Bug 14670: Add Cite-option to detail page in OPAC Adds a "cite" option in the tools menu in OPAC detail page for easy citations. Test plan: 1. Apply this patch 2. Got to OPAC detail page 3. In the tools menu to the right, click the new "Cite" link 4. A modal should appear with four different citation styles (APA, Chicago, Harvard, MLA) Sponsored-by: Regionbibliotek Halland / County library of Halland Signed-off-by: Nicole Engard <nengard@bywatersolutions.com> I love it, I'd recommend adding an icon to the menu next to it, but this patch works as described so I'm signing off.
QA comments: 1/ New subroutines have to be covered by tests 2/ You should provide the related change for opac.less Marked as Failed QA.
Created attachment 43770 [details] [review] Bug 14670: add related css changes to opac.less
Created attachment 43771 [details] [review] Bug 14670: add test for C4::Record::marc2cites
Hi Martin, it's usually ok to switch back to signed off once you have provided patches fixing the QA issues - this way it will get reviewed again.
Please launch the QA script before submitting patches: FAIL C4/Record.pm FAIL critic # ControlStructures::ProhibitMutatingListFunctions: Got 1 violation(s). FAIL valid "my" variable @authors masks earlier declaration in same scope use perl -wc and perlcritic to get more info. There is something wrong with the css/less files, but it already exists on master.
Sorry about that, didn't know about the QA test tool. Issues are fixed, attaching two patches below. What is wrong with the css/less? Is it something introduced with any of the 14670 patches?
Created attachment 43828 [details] [review] Bug 14670: Fix earlier declaration in same scope
Created attachment 43829 [details] [review] Bug 14670: fix ControlStructures::ProhibitMutatingListFunctions
I don't feel like this is ready to be pushed quite yet. I will give some examples: 1) If a record (serials?) don't have a publication year, the styles will look like this: APA (). Handelsblatt. Düsseldorf: Verl.-Gruppe Handelsblatt. Chicago . Handelsblatt. Düsseldorf: Verl.-Gruppe Handelsblatt. Harvard (). Handelsblatt. Düsseldorf: Verl.-Gruppe Handelsblatt. MLA Handelsblatt. Düsseldorf: Verl.-Gruppe Handelsblatt. . So it doesn't cover for the case the publication year doesn't exist in the record. Another case are articles - the citation will miss the host item information, which is very vital for those: APA Schweizer K., . (2010). 100 Jahre Internationaler Frauentag. : . Chicago Schweizer Karl, . 2010. 100 Jahre Internationaler Frauentag. : . Harvard Schweizer K., . (2010). 100 Jahre Internationaler Frauentag. : . MLA Schweizer Karl, . 100 Jahre Internationaler Frauentag. : . 2010. I think this needs more testing with different type of media. Also one small thing, that I had fixed for a follow-up, but will put here as a comment now: - place => $record->subfield("260", "a") || "", + place => $record->subfield("264", "a") || $record->subfield("260", "a") || "", publisher => $record->subfield("264", "b") || $record->subfield("260", "b") || "", Last but not least: I am reattaching the patches with a follow up taking care of a conflict in the opac.css. I have removed it from the first patch and put the fixed file into a separate patch.
Created attachment 43919 [details] [review] Bug 14670: Add Cite-option to detail page in OPAC Adds a "cite" option in the tools menu in OPAC detail page for easy citations. Test plan: 1. Apply this patch 2. Got to OPAC detail page 3. In the tools menu to the right, click the new "Cite" link 4. A modal should appear with four different citation styles (APA, Chicago, Harvard, MLA) Sponsored-by: Regionbibliotek Halland / County library of Halland Signed-off-by: Nicole Engard <nengard@bywatersolutions.com>
Created attachment 43920 [details] [review] Bug 14670: add related css changes to opac.less
Created attachment 43921 [details] [review] Bug 14670: add test for C4::Record::marc2cites
Created attachment 43922 [details] [review] Bug 14670: Fix earlier declaration in same scope
Created attachment 43923 [details] [review] Bug 14670: fix ControlStructures::ProhibitMutatingListFunctions
Created attachment 43924 [details] [review] Bug 14670: QA follow-up: Regenerated opac.css The first patch originally contained the changes opac.css. I removed the file to fix the conflict. This patch readds the opac.css changes generated from the changes to the .less file.
Thanks for all the feedback and help! We sort of hoped that this would be more of a good "warm up" patch and with time running out for Martin in our project I have to point him in other directions so we're abandoning this one. We may be able to pick it up later, but I don't dare to make any such promises.
I'm not sure why this has been marked Failed QA - just tested and works well. I think the only thing it needs is an icon in the menu like the other menu options have! Anyone still working on this?
Hi Aleisha, have you seen my comment#11 with the examples?
(In reply to Aleisha Amohia from comment #19) > I'm not sure why this has been marked Failed QA - just tested and works > well. I think the only thing it needs is an icon in the menu like the other > menu options have! Anyone still working on this? We´re not currently working on this. There´s still a few hundred developer hours left in our project, but for now we'll have to focus on things that have higher priority. Anyone is welcome to take it further, but I'll keep following the thread and at least keep it in mind if we should have developer hours left once we get to lower priority development.
Created attachment 75679 [details] [review] Bug 14670: Add Cite-option to detail page in OPAC Adds a "cite" option in the tools menu in OPAC detail page for easy citations. Test plan: 1. Apply this patch 2. Got to OPAC detail page 3. In the tools menu to the right, click the new "Cite" link 4. A modal should appear with four different citation styles (APA, Chicago, Harvard, MLA) Sponsored-by: Regionbibliotek Halland / County library of Halland Signed-off-by: Nicole Engard <nengard@bywatersolutions.com>
Created attachment 75680 [details] [review] Bug 14670: add related css changes to opac.less
Created attachment 75681 [details] [review] Bug 14670: add test for C4::Record::marc2cites
Created attachment 75682 [details] [review] Bug 14670: Fix earlier declaration in same scope
Created attachment 75683 [details] [review] Bug 14670: fix ControlStructures::ProhibitMutatingListFunctions
I rebased(In reply to Viktor Sarge from comment #21) > We´re not currently working on this. I have attached rebased patches in case someone wants to pick this up. It certainly looks like it's close.
Created attachment 161600 [details] [review] Bug 14670: Add Cite-option to detail page in OPAC Adds a "cite" option in the tools menu in OPAC detail page for easy citations. Test plan: 1. Apply this patch 2. Got to OPAC detail page 3. In the tools menu to the right, click the new "Cite" link 4. A modal should appear with four different citation styles (APA, Chicago, Harvard, MLA) Sponsored-by: Regionbibliotek Halland / County library of Halland Signed-off-by: Nicole Engard <nengard@bywatersolutions.com>
Created attachment 161601 [details] [review] Bug 14670: add test for C4::Record::marc2cites
Created attachment 161602 [details] [review] Bug 14670: Fix earlier declaration in same scope
Created attachment 161603 [details] [review] Bug 14670: fix ControlStructures::ProhibitMutatingListFunctions
Created attachment 161604 [details] [review] Bug 14670: Move modal code to an include file Also add an icon for the "Cite" entry. Sponsored-by: Orex Digital
Created attachment 161605 [details] [review] Bug 14670: Deal with empty publicatoin date Sponsored-by: Orex Digital
Created attachment 161606 [details] [review] Bug 14670: Use 260 if 264 is empty Sponsored-by: Orex Digital
Created attachment 161607 [details] [review] Bug 14670: Fix tests and import Sponsored-by: Orex Digital
Patches rebased and added some follow-ups. I think this is not ready yet. The code in C4::Record needs to be beautify a bit and more tests. Also I am wondering if we should not have a syspref. Patches for discussion, get feedback and revive this bug.
The patches still applied and everything looks OK to me. Would people want a copy button next to each citation? Although easy enough to select and copy. Wasn't sure whether this should be signed off, or whether you would like more extensive feedback. As I'm not an academic, or familiar with the citation requirements, I'm probably not the best person to provide it!
Thanks, David. I am actually expecting a signoff from the sponsor to make sure they are happy with the behaviour.
Created attachment 166474 [details] [review] Bug 14670: Fix tests and import Sponsored-by: Orex Digital Signed-off-by: alexandre <alexandre.noel@inlibro.com>
Created attachment 167528 [details] [review] Bug 14670: Add Cite-option to detail page in OPAC Adds a "cite" option in the tools menu in OPAC detail page for easy citations. Test plan: 1. Apply this patch 2. Got to OPAC detail page 3. In the tools menu to the right, click the new "Cite" link 4. A modal should appear with four different citation styles (APA, Chicago, Harvard, MLA) Sponsored-by: Regionbibliotek Halland / County library of Halland Signed-off-by: Nicole Engard <nengard@bywatersolutions.com> Signed-off-by: hugo <hagud@orex.es>
Created attachment 167529 [details] [review] Bug 14670: add test for C4::Record::marc2cites Signed-off-by: hugo <hagud@orex.es>
Created attachment 167530 [details] [review] Bug 14670: Fix earlier declaration in same scope Signed-off-by: hugo <hagud@orex.es>
Created attachment 167531 [details] [review] Bug 14670: fix ControlStructures::ProhibitMutatingListFunctions Signed-off-by: hugo <hagud@orex.es>
Created attachment 167532 [details] [review] Bug 14670: Move modal code to an include file Also add an icon for the "Cite" entry. Sponsored-by: Orex Digital Signed-off-by: hugo <hagud@orex.es>
Created attachment 167533 [details] [review] Bug 14670: Deal with empty publicatoin date Sponsored-by: Orex Digital Signed-off-by: hugo <hagud@orex.es>
Created attachment 167534 [details] [review] Bug 14670: Use 260 if 264 is empty Sponsored-by: Orex Digital Signed-off-by: hugo <hagud@orex.es>
Created attachment 167535 [details] [review] Bug 14670: Fix tests and import Sponsored-by: Orex Digital Signed-off-by: alexandre <alexandre.noel@inlibro.com> Signed-off-by: hugo <hagud@orex.es>
It's a shame there's not a CSL lib for Perl, else I'd have loved to see this support that instead of hard coding citation styles. But.. this isn't a bad first step and is long overdue in Koha really. https://citationstyles.org/developers/
Created attachment 173515 [details] [review] Bug 14670: Add Cite-option to detail page in OPAC Adds a "cite" option in the tools menu in OPAC detail page for easy citations. Test plan: 1. Apply this patch 2. Got to OPAC detail page 3. In the tools menu to the right, click the new "Cite" link 4. A modal should appear with four different citation styles (APA, Chicago, Harvard, MLA) Sponsored-by: Regionbibliotek Halland / County library of Halland Signed-off-by: Nicole Engard <nengard@bywatersolutions.com> Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173516 [details] [review] Bug 14670: add test for C4::Record::marc2cites Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173517 [details] [review] Bug 14670: Fix earlier declaration in same scope Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173518 [details] [review] Bug 14670: fix ControlStructures::ProhibitMutatingListFunctions Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173519 [details] [review] Bug 14670: Move modal code to an include file Also add an icon for the "Cite" entry. Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173520 [details] [review] Bug 14670: Deal with empty publicatoin date Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173521 [details] [review] Bug 14670: Use 260 if 264 is empty Sponsored-by: Orex Digital Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173522 [details] [review] Bug 14670: Fix tests and import Sponsored-by: Orex Digital Signed-off-by: Alexandre Noel <alexandre.noel@inlibro.com> Signed-off-by: Hugo Agud <hagud@orex.es> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173523 [details] [review] Bug 14670: (QA follow-up) Add missing filter Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 173524 [details] [review] Bug 14670: (QA follow-up) Tidy new code Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Whilst this is very old style code, it's functional and was first submitted in 2015! I'd love to see export to RIS added as a button to the Modal (RIS is the standard for exporting data to citation management programs and is already supported in our bib export options) I'd also love to see this converted to using citeproc-js to allow csl profiles to be used for whatever citation style your institution mandates. But.. none of that is a fail for me.
I am sorry, I just tested this on main and it's not working for me: applied patch yarn build restart_all open OPAC in new private window Cit is visible in the detail view, but clicking on it just scrolls up the page, there is no modal. :(
This is not actually a big feature. I think the reason for the modal trouble could be small (or even me) - giving this one another chance until middle of the week.
Created attachment 174453 [details] [review] Bug 14670: Adjustments for Bootstrap 5
Pushed for 24.11! Well done everyone, thank you!