Description
Martin Renvoize (ashimema)
2023-10-20 14:34:57 UTC
Created attachment 157529 [details] [review] Bug 35119: Use page-section and bg-danger on biblio errors This patch uses the page-section and bg-danger classes to increase the prominence of bibliographic record errors in the staff client to encourage staff to fix such issues as soon as possible. Test plan 1) Using ktd navigate to record 369 2) Note the display of an encoding warning at the top of the page 3) Apply the patch 4) Refresh the above page and note the stronger visual prominence of the error I feel like we should be able to do better with the encode error itself too.. it's all on one line as it displays right now.. Created attachment 157538 [details] [review] Bug 35119: Use page-section and bg-danger on biblio errors This patch uses the page-section and bg-danger classes to increase the prominence of bibliographic record errors in the staff client to encourage staff to fix such issues as soon as possible. We also output the actual encoding error in a <pre> block to maintain the proper whitespacing of the errors themselves.. this makes it much clearer which characters are the issues. Test plan 1) Using ktd navigate to record 369 2) Note the display of an encoding warning at the top of the page 3) Apply the patch 4) Refresh the above page and note the stronger visual prominence of the error Much better.. the errors are now super clear! Created attachment 157553 [details] [review] Bug 35119: Use page-section and bg-danger on biblio errors This patch uses the page-section and bg-danger classes to increase the prominence of bibliographic record errors in the staff client to encourage staff to fix such issues as soon as possible. We also output the actual encoding error in a <pre> block to maintain the proper whitespacing of the errors themselves.. this makes it much clearer which characters are the issues. Test plan 1) Using ktd navigate to record 369 2) Note the display of an encoding warning at the top of the page 3) Apply the patch 4) Refresh the above page and note the stronger visual prominence of the error Signed-off-by: David Nind <david@davidnind.com> Created attachment 157561 [details] [review] Bug 35119: (follow-up) Same treatment for editor page We pushed 34014 before I thought about these changes.. I think this is a solid follow-up to add the same styling to the catalogue editor page.. swapping the existing dialog alert with a page-section bg-danger and thus allowing more space to 'pre' wrap the encoding error itself to ease the spotting of errant characters. Created attachment 157577 [details] [review] Bug 35119: Use page-section and bg-danger on biblio errors This patch uses the page-section and bg-danger classes to increase the prominence of bibliographic record errors in the staff client to encourage staff to fix such issues as soon as possible. We also output the actual encoding error in a <pre> block to maintain the proper whitespacing of the errors themselves.. this makes it much clearer which characters are the issues. Test plan 1) Using ktd navigate to record 369 2) Note the display of an encoding warning at the top of the page 3) Apply the patch 4) Refresh the above page and note the stronger visual prominence of the error Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Created attachment 157578 [details] [review] Bug 35119: (follow-up) Same treatment for editor page We pushed 34014 before I thought about these changes.. I think this is a solid follow-up to add the same styling to the catalogue editor page.. swapping the existing dialog alert with a page-section bg-danger and thus allowing more space to 'pre' wrap the encoding error itself to ease the spotting of errant characters. Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Tempting to say we should amend the message shown in the advanced editor to something like "please see logs, or just switch to the basic editor where we'll actually show you the errors." Created attachment 157641 [details] [review] Bug 35119: (follow-up) Adapt advanced editor message I'd love to have included the error details within the message popup, but that alluded me so this patch just updates the message to hint that details can be seen on the basic editor screen. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 157643 [details] [review] Bug 35119: Use page-section and bg-danger on biblio errors This patch uses the page-section and bg-danger classes to increase the prominence of bibliographic record errors in the staff client to encourage staff to fix such issues as soon as possible. We also output the actual encoding error in a <pre> block to maintain the proper whitespacing of the errors themselves.. this makes it much clearer which characters are the issues. Test plan 1) Using ktd navigate to record 369 2) Note the display of an encoding warning at the top of the page 3) Apply the patch 4) Refresh the above page and note the stronger visual prominence of the error Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 157644 [details] [review] Bug 35119: (follow-up) Same treatment for editor page We pushed 34014 before I thought about these changes.. I think this is a solid follow-up to add the same styling to the catalogue editor page.. swapping the existing dialog alert with a page-section bg-danger and thus allowing more space to 'pre' wrap the encoding error itself to ease the spotting of errant characters. Signed-off-by: Phil Ringnalda <phil@chetcolibrary.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 157645 [details] [review] Bug 35119: (follow-up) Adapt advanced editor message I'd love to have included the error details within the message popup, but that alluded me so this patch just updates the message to hint that details can be seen on the basic editor screen. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Pushed to master for 23.11. Nice work everyone, thanks! Depends on Bug 35099 not in 23.05.x This seems to have broken some selenium tests. Please fix ASAP. https://jenkins.koha-community.org/job/Koha_Master/lastCompletedBuild/testReport/ Created attachment 157984 [details] [review] Bug 35119: Add back classes used for selenium tests This patch re-instates the classes used for the selenium tests.. I checked we weren't using them for javascript or css, but didn't think about tests before. Created attachment 157985 [details] [review] Bug 35119: Add back classes used for selenium tests This patch re-instates the classes used for the selenium tests.. I checked we weren't using them for javascript or css, but didn't think about tests before. Follow-up pushed to master. Thanks! |