Description
Owen Leonard
2024-04-09 14:40:40 UTC
Created attachment 164563 [details] [review] Bug 36166: Convert Cart and Lists conrols to buttons and rework logic This patch tries to address errors in the way features are displayed in result lists so that controls for holds, tags, lists, etc. are shown or hidden according to system preferences. The patch converst the Cart/Lists dropdown to separate buttons, making the display logic simpler and making the interface more consistent with updates to the staff interface search results. To test, apply the patch and rebuild the OPAC CSS. - In the OPAC, test these pages: - Catalog search results - List contents - Test with various combinations of these system preferences: - opacbookbag - DisplayMultiPlaceHold - virtualshelves - OPACHoldRequests - TagsEnabled - TagsInputOnList - UseRecalls - ArticleRequests With each different combination of settings the right controls should appear in the toolbar at the top (if present), and with each search result. (In reply to David Nind from comment #2) > Requires bug 32607, which currently no longer applies. That should have been bug 36207! That bug was fixed. However, now I'm getting a sha1/fake ancestor error for bug 36166. (In reply to David Nind from comment #3) > However, now I'm getting a sha1/fake ancestor error for bug 36166. If I apply bug 36166 by itself, everything works as it should - no sha1/fake ancestor error. However, I think I worked out what the problem is. The sha1/fake ancestor is for this bug - the patch itself for this bug has the wrong bug number! - Bug title: Bug 36557 - Improve logic and display of OPAC cart, tag, and lists controls - Bug attached: Bug 36166: Convert Cart and Lists conrols to buttons and rework logic () Here is the sha1/fake ancestor error: git bz apply 36557 Bug 36557 Depends on bug 36166 (Signed Off) Follow? [(y)es, (n)o] y Bug 36166 Depends on bug 36207 (Signed Off) Follow? [(y)es, (n)o] y Bug 36207 - Convert browser alerts to modals: OPAC tags 166212 - Bug 36207: Use confirmation modal when removing tags from titles in the OPAC Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 36207: Use confirmation modal when removing tags from titles in the OPAC Bug 36166 - Disable select to add to list if opacuserlogin is disabled 162794 - Bug 36166: Disable select to add to list if opacuserlogin is disabled Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 36166: Disable select to add to list if opacuserlogin is disabled Bug 36557 - Improve logic and display of OPAC cart, tag, and lists controls 164563 - Bug 36166: Convert Cart and Lists conrols to buttons and rework logic Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 36166: Convert Cart and Lists conrols to buttons and rework logic error: sha1 information is lacking or useless (koha-tmpl/opac-tmpl/bootstrap/css/src/opac.scss). error: could not build fake ancestor Patch failed at 0001 Bug 36166: Convert Cart and Lists conrols to buttons and rework logic Created attachment 166297 [details] [review] Bug 36166: Convert Cart and Lists controls to buttons and rework logic This patch tries to address errors in the way features are displayed in result lists so that controls for holds, tags, lists, etc. are shown or hidden according to system preferences. The patch converts the Cart/Lists dropdown to separate buttons, making the display logic simpler and making the interface more consistent with updates to the staff interface search results. To test, apply the patch and rebuild the OPAC CSS. - In the OPAC, test these pages: - Catalog search results - List contents - Test with various combinations of these system preferences: - opacbookbag - DisplayMultiPlaceHold - virtualshelves - OPACHoldRequests - TagsEnabled - TagsInputOnList - UseRecalls - ArticleRequests With each different combination of settings the right controls should appear in the toolbar at the top (if present), and with each search result. Getting a really weird error now when trying to apply the patch 8-(... git bz apply 36557 Bug 36557 Depends on bug 36166 (Signed Off) Follow? [(y)es, (n)o] y Bug 36166 Depends on bug 36207 (Signed Off) Follow? [(y)es, (n)o] y Bug 36207 - Convert browser alerts to modals: OPAC tags 166212 - Bug 36207: Use confirmation modal when removing tags from titles in the OPAC Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 36207: Use confirmation modal when removing tags from titles in the OPAC Bug 36166 - Disable select to add to list if opacuserlogin is disabled 162794 - Bug 36166: Disable select to add to list if opacuserlogin is disabled Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 36166: Disable select to add to list if opacuserlogin is disabled Bug 36557 - Improve logic and display of OPAC cart, tag, and lists controls 166297 - Bug 36166: Convert Cart and Lists controls to buttons and rework logic Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 36166: Convert Cart and Lists controls to buttons and rework logic Moving to starting point Adding bug reference 825098d Bug 36166: Convert Cart and Lists controls to buttons and rework logic Cleaning up back to original state on error Traceback (most recent call last): File "/usr/bin/git-bz", line 2834, in <module> applied = do_apply(bug_ref) ^^^^^^^^^^^^^^^^^ File "/usr/bin/git-bz", line 1977, in do_apply add_url(bug, commits) File "/usr/bin/git-bz", line 1719, in add_url add_url_to_head_commit(commit, bug) File "/usr/bin/git-bz", line 1682, in add_url_to_head_commit git.commit(file="-", amend=True, _input=input) File "/usr/bin/git-bz", line 197, in f o = git_run(command, *args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/bin/git-bz", line 167, in git_run output, error = process.communicate(input) ^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/subprocess.py", line 1207, in communicate stdout, stderr = self._communicate(input, endtime, timeout) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/subprocess.py", line 2039, in _communicate input_view = memoryview(self._input) ^^^^^^^^^^^^^^^^^^^^^^^ TypeError: memoryview: a bytes-like object is required, not 'str' It looks like a git bz issue, but not spotting what's actually going on. Adding Joubu. Bug number on the patch is not 36557, and the git-bz python3 version is buggy... Created attachment 166422 [details] [review] Bug 36557: Convert Cart and Lists controls to buttons and rework logic This patch tries to address errors in the way features are displayed in result lists so that controls for holds, tags, lists, etc. are shown or hidden according to system preferences. The patch converts the Cart/Lists dropdown to separate buttons, making the display logic simpler and making the interface more consistent with updates to the staff interface search results. To test, apply the patch and rebuild the OPAC CSS. - In the OPAC, test these pages: - Catalog search results - List contents - Test with various combinations of these system preferences: - opacbookbag - DisplayMultiPlaceHold - virtualshelves - OPACHoldRequests - TagsEnabled - TagsInputOnList - UseRecalls - ArticleRequests With each different combination of settings the right controls should appear in the toolbar at the top (if present), and with each search result. Thanks for fixing! The patch now applies. The only thing that wasn't working as expected for me was article requests. With ArticleRequests enabled (as well as ArticleRequestsLinkControl set to 'Use algorithm to show or hide'): 1. Before the patch, 'Request article' was displayed in the actions for each result in the search results and on the record details page. It was also displayed on the list results. 2. After the patch, 'Request article' is not displayed in the search results, record details page, or lists. Not sure whether this was intended, or if it has something to do with ArticleRequestsLinkControl. With that set to always show, 'Request article' appears for all search results and on lists, but not on the record details page. (In reply to David Nind from comment #10) > 2. After the patch, 'Request article' is not displayed in the search > results, record details page, or lists. I'm not seeing this. In my testing the 'Request article' link shows up as expected on search results and lists with both settings of ArticleRequestsLinkControl. My patch doesn't touch the record detail page, so if there's an issue with that it's separate. (In reply to Owen Leonard from comment #11) > (In reply to David Nind from comment #10) > > > 2. After the patch, 'Request article' is not displayed in the search > > results, record details page, or lists. > > I'm not seeing this. In my testing the 'Request article' link shows up as > expected on search results and lists with both settings of > ArticleRequestsLinkControl. > > My patch doesn't touch the record detail page, so if there's an issue with > that it's separate. Have re-tested and is now showing. My apologies Owen, there must have been something wrong with my setup.... Created attachment 166798 [details] [review] Bug 36557: Convert Cart and Lists controls to buttons and rework logic This patch tries to address errors in the way features are displayed in result lists so that controls for holds, tags, lists, etc. are shown or hidden according to system preferences. The patch converts the Cart/Lists dropdown to separate buttons, making the display logic simpler and making the interface more consistent with updates to the staff interface search results. To test, apply the patch and rebuild the OPAC CSS. - In the OPAC, test these pages: - Catalog search results - List contents - Test with various combinations of these system preferences: - opacbookbag - DisplayMultiPlaceHold - virtualshelves - OPACHoldRequests - TagsEnabled - TagsInputOnList - UseRecalls - ArticleRequests With each different combination of settings the right controls should appear in the toolbar at the top (if present), and with each search result. Signed-off-by: David Nind <david@davidnind.com> Created attachment 167966 [details] [review] Bug 36557: (follow-up) Restore missing "Remove from list" control This patch restores the "Remove from list" control to the batch operations toolbar when one is looking at the contents of a list. It was unintentionally left out of the previous patch. The patch also corrects some button markup to ensure consisten style of controls in the batch operations toolbar. Created attachment 167997 [details] [review] Bug 36557: Convert Cart and Lists controls to buttons and rework logic This patch tries to address errors in the way features are displayed in result lists so that controls for holds, tags, lists, etc. are shown or hidden according to system preferences. The patch converts the Cart/Lists dropdown to separate buttons, making the display logic simpler and making the interface more consistent with updates to the staff interface search results. To test, apply the patch and rebuild the OPAC CSS. - In the OPAC, test these pages: - Catalog search results - List contents - Test with various combinations of these system preferences: - opacbookbag - DisplayMultiPlaceHold - virtualshelves - OPACHoldRequests - TagsEnabled - TagsInputOnList - UseRecalls - ArticleRequests With each different combination of settings the right controls should appear in the toolbar at the top (if present), and with each search result. Signed-off-by: David Nind <david@davidnind.com> Created attachment 167998 [details] [review] Bug 36557: (follow-up) Restore missing "Remove from list" control This patch restores the "Remove from list" control to the batch operations toolbar when one is looking at the contents of a list. It was unintentionally left out of the previous patch. The patch also corrects some button markup to ensure consisten style of controls in the batch operations toolbar. Signed-off-by: David Nind <david@davidnind.com> Sorry Owen.. I'm getting the dreaded sha1 apply error here Created attachment 168248 [details] [review] Bug 36557: Convert Cart and Lists controls to buttons and rework logic This patch tries to address errors in the way features are displayed in result lists so that controls for holds, tags, lists, etc. are shown or hidden according to system preferences. The patch converts the Cart/Lists dropdown to separate buttons, making the display logic simpler and making the interface more consistent with updates to the staff interface search results. To test, apply the patch and rebuild the OPAC CSS. - In the OPAC, test these pages: - Catalog search results - List contents - Test with various combinations of these system preferences: - opacbookbag - DisplayMultiPlaceHold - virtualshelves - OPACHoldRequests - TagsEnabled - TagsInputOnList - UseRecalls - ArticleRequests With each different combination of settings the right controls should appear in the toolbar at the top (if present), and with each search result. Signed-off-by: David Nind <david@davidnind.com> Created attachment 168249 [details] [review] Bug 36557: (follow-up) Restore missing "Remove from list" control This patch restores the "Remove from list" control to the batch operations toolbar when one is looking at the contents of a list. It was unintentionally left out of the previous patch. The patch also corrects some button markup to ensure consisten style of controls in the batch operations toolbar. Signed-off-by: David Nind <david@davidnind.com> Created attachment 168251 [details] [review] Bug 36557: Convert Cart and Lists controls to buttons and rework logic This patch tries to address errors in the way features are displayed in result lists so that controls for holds, tags, lists, etc. are shown or hidden according to system preferences. The patch converts the Cart/Lists dropdown to separate buttons, making the display logic simpler and making the interface more consistent with updates to the staff interface search results. To test, apply the patch and rebuild the OPAC CSS. - In the OPAC, test these pages: - Catalog search results - List contents - Test with various combinations of these system preferences: - opacbookbag - DisplayMultiPlaceHold - virtualshelves - OPACHoldRequests - TagsEnabled - TagsInputOnList - UseRecalls - ArticleRequests With each different combination of settings the right controls should appear in the toolbar at the top (if present), and with each search result. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 168252 [details] [review] Bug 36557: (follow-up) Restore missing "Remove from list" control This patch restores the "Remove from list" control to the batch operations toolbar when one is looking at the contents of a list. It was unintentionally left out of the previous patch. The patch also corrects some button markup to ensure consisten style of controls in the batch operations toolbar. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Logic all checks out, looks nicer. Thanks for the thorough testing David. QA scripts all happy. Passing QA Pushed for 24.11! Well done everyone, thank you! Backported to 24.05.x for upcoming 24.05.06 Dependancies not in 23.11.x |