|
Description
Owen Leonard
2020-03-31 21:35:18 UTC
Created attachment 102162 [details] [review] Bug 25031: Improve handling of multiple covers on the biblio detail page in the staff client This patch modifies the template, JS, and CSS for the bibliographic detail page in order to gracefully handle multiple cover images. The changed version loops through any cover images which might be embedded and checks that they are successfully loaded. Only successfully-loaded images are shown. Only the first image is shown, and the others can be "paged through" using generated navigation controls. To test, apply the page and rebuild the staff client CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client). Enable multiple cover image services. The patch was developed with these services available: - Amazon - Local cover images (including multiple local cover images) - Coce (serving up Amazon, Google, and OpenLibrary images) - Images from the CustomCoverImages preference View a variety of titles and confirm that the cover images are displaying correctly, whether there be 0, 1, 2, or more covers available. I have tested with Amazon and local cover images, and it seems to work okay. I don't have access to a Coce server, so I wasn't able to test that. I did note that if there is no image available, then there is a visible border but with nothing inside of it. Created attachment 102295 [details] [review] Bug 25031: Improve handling of multiple covers on the biblio detail page in the staff client This patch modifies the template, JS, and CSS for the bibliographic detail page in order to gracefully handle multiple cover images. The changed version loops through any cover images which might be embedded and checks that they are successfully loaded. Only successfully-loaded images are shown. Only the first image is shown, and the others can be "paged through" using generated navigation controls. To test, apply the page and rebuild the staff client CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client). Enable multiple cover image services. The patch was developed with these services available: - Amazon - Local cover images (including multiple local cover images) - Coce (serving up Amazon, Google, and OpenLibrary images) - Images from the CustomCoverImages preference View a variety of titles and confirm that the cover images are displaying correctly, whether there be 0, 1, 2, or more covers available. (In reply to David Nind from comment #2) > I did note that if there is no image available, then there is a visible > border but with nothing inside of it. This is now fixed. Thanks Owen! Created attachment 103040 [details] [review] Bug 25031: Improve handling of multiple covers on the biblio detail page in the staff client This patch modifies the template, JS, and CSS for the bibliographic detail page in order to gracefully handle multiple cover images. The changed version loops through any cover images which might be embedded and checks that they are successfully loaded. Only successfully-loaded images are shown. Only the first image is shown, and the others can be "paged through" using generated navigation controls. To test, apply the page and rebuild the staff client CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client). Enable multiple cover image services. The patch was developed with these services available: - Amazon - Local cover images (including multiple local cover images) - Coce (serving up Amazon, Google, and OpenLibrary images) - Images from the CustomCoverImages preference View a variety of titles and confirm that the cover images are displaying correctly, whether there be 0, 1, 2, or more covers available. Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> I love this patch. 1. Should not we hide the green dot if there is only 1 image? 2. Clicking a blue dot (select another image), the scrollbar is hitting the top (preventDefault is missing?) 3. Without cover images, I see the border of #cover-slides, then disappears. Cannot we avoid that? Created attachment 103079 [details] [review] Bug 25031: (QA follow-up) Improve handling of one or fewer images This patch addresses a few issues raised during QA: 1. If there are no images, avoid the momentary appearance of an empty borered box. The class controlling the appearance of the box is now added after initialization. 2. If there is only one image, remove the control for switching between covers. 3. Add "preventDefault" to cover naviation click handler. 4. Correct translation function in localcovers.js. This patch modifies SCSS, so rebuilding the staff client CSS is necessary for testing. Marking failed QA because the reliability of this patch's intended behavior seems to be problematic. Created attachment 105986 [details] [review] Bug 25031: Improve handling of multiple covers on the biblio detail page in the staff client This patch modifies the template, JS, and CSS for the bibliographic detail page in order to gracefully handle multiple cover images. The changed version loops through any cover images which might be embedded and checks that they are successfully loaded. Only successfully-loaded images are shown. Only the first image is shown, and the others can be "paged through" using generated navigation controls. To test, apply the page and rebuild the staff client CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client). Enable multiple cover image services. The patch was developed with these services available: - Amazon - Local cover images (including multiple local cover images) - Coce (serving up Amazon, Google, and OpenLibrary images) - Images from the CustomCoverImages preference View a variety of titles and confirm that the cover images are displaying correctly, whether there be 0, 1, 2, or more covers available. Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Created attachment 105987 [details] [review] Bug 25031: (QA follow-up) Improve handling of one or fewer images This patch addresses a few issues raised during QA: 1. If there are no images, avoid the momentary appearance of an empty borered box. The class controlling the appearance of the box is now added after initialization. 2. If there is only one image, remove the control for switching between covers. 3. Add "preventDefault" to cover naviation click handler. 4. Correct translation function in localcovers.js. This patch modifies SCSS, so rebuilding the staff client CSS is necessary for testing. I really like this, would love to see results changed as well! Created attachment 106612 [details] [review] Bug 25031: Improve handling of multiple covers on the biblio detail page in the staff client This patch modifies the template, JS, and CSS for the bibliographic detail page in order to gracefully handle multiple cover images. The changed version loops through any cover images which might be embedded and checks that they are successfully loaded. Only successfully-loaded images are shown. Only the first image is shown, and the others can be "paged through" using generated navigation controls. To test, apply the page and rebuild the staff client CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client). Enable multiple cover image services. The patch was developed with these services available: - Amazon - Local cover images (including multiple local cover images) - Coce (serving up Amazon, Google, and OpenLibrary images) - Images from the CustomCoverImages preference View a variety of titles and confirm that the cover images are displaying correctly, whether there be 0, 1, 2, or more covers available. Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 106613 [details] [review] Bug 25031: (QA follow-up) Improve handling of one or fewer images This patch addresses a few issues raised during QA: 1. If there are no images, avoid the momentary appearance of an empty borered box. The class controlling the appearance of the box is now added after initialization. 2. If there is only one image, remove the control for switching between covers. 3. Add "preventDefault" to cover naviation click handler. 4. Correct translation function in localcovers.js. This patch modifies SCSS, so rebuilding the staff client CSS is necessary for testing. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> I have enabled: AdlibrisCoversEnabled AmazonCoverImages GoogleJackets AllowMultipleCovers CustomCoverImages LocalCoverImages OPACLocalCoverImages Use biblionumber=22 from misc4dev I see "Image from Adlibris" and one image Edit, attach image, select an image I now see 3 dots, first is the local image, second is adlibris: https://snipboard.io/CdOwKt.jpg Third is... I don't know! https://snipboard.io/gL8wev.jpg Created attachment 107272 [details] [review] Bug 25031: (QA follow-up) Avoid displaying extra navigation This patch reworks the construction of the navigation links in such a way that they are built during a loop over an updated jQuery object. Previously the stored variable might not reflect that some elements had been removed. Not considering this blocker but may be good to fix: With more than 1 cover: https://snipboard.io/MC8sRx.jpg With 1 cover: https://snipboard.io/fqz60Y.jpg I'd say we should add a bit of margin-top if only 1 cover. Please attach a follow-up here if it can be fixed easily. I found a blocker bug: reset_all (no cover image prefs are set) Go to the detail page => None of the links from #catalogue_detail_biblio are clickable (like there is a layer on top of it) If I turn on LocalCoverImages, the links work (!) Created attachment 107310 [details] [review] Bug 25031: (follow-up) Fix markup error and restore cover class This patch corrects a markup error that appears when no cover image services are enabled. An unclosed <div> caused links on the page to break. This patch also restores a part of the cover-handling script which adds a bordered style to the cover image "tiles" Pushed to master for 20.11, thanks to everybody involved! This is awesome but I will not backport the enhancement to 20.05.x *** Bug 25924 has been marked as a duplicate of this bug. *** |
