Bug 35119 - Make bibliographic encoding errors more prominent and match current styling
Summary: Make bibliographic encoding errors more prominent and match current styling
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Nick Clemens (kidclamp)
URL:
Keywords:
Depends on: 34014 35099
Blocks:
  Show dependency treegraph
 
Reported: 2023-10-20 14:34 UTC by Martin Renvoize
Modified: 2023-11-30 13:07 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00


Attachments
Bug 35119: Use page-section and bg-danger on biblio errors (2.74 KB, patch)
2023-10-20 14:37 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35119: Use page-section and bg-danger on biblio errors (2.89 KB, patch)
2023-10-20 15:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35119: Use page-section and bg-danger on biblio errors (2.94 KB, patch)
2023-10-20 15:13 UTC, David Nind
Details | Diff | Splinter Review
Bug 35119: (follow-up) Same treatment for editor page (2.15 KB, patch)
2023-10-20 18:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35119: Use page-section and bg-danger on biblio errors (2.99 KB, patch)
2023-10-20 20:49 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 35119: (follow-up) Same treatment for editor page (2.20 KB, patch)
2023-10-20 20:49 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 35119: (follow-up) Adapt advanced editor message (1.50 KB, patch)
2023-10-23 09:56 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35119: Use page-section and bg-danger on biblio errors (3.04 KB, patch)
2023-10-23 12:19 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 35119: (follow-up) Same treatment for editor page (2.25 KB, patch)
2023-10-23 12:19 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 35119: (follow-up) Adapt advanced editor message (1.55 KB, patch)
2023-10-23 12:19 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 35119: Add back classes used for selenium tests (1.69 KB, patch)
2023-10-27 14:47 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 35119: Add back classes used for selenium tests (2.57 KB, patch)
2023-10-27 14:50 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2023-10-20 14:34:57 UTC
Currently, the encoding errors and analytics search error warnings are outside of the normal page flow and they could be more prominent using a bg-warning coloured page-section.
Comment 1 Martin Renvoize 2023-10-20 14:37:54 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
Comment 2 Martin Renvoize 2023-10-20 15:01:09 UTC
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..
Comment 3 Martin Renvoize 2023-10-20 15:04:18 UTC
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
Comment 4 Martin Renvoize 2023-10-20 15:06:14 UTC
Much better.. the errors are now super clear!
Comment 5 David Nind 2023-10-20 15:13:06 UTC
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>
Comment 6 Martin Renvoize 2023-10-20 18:21:44 UTC
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.
Comment 7 Phil Ringnalda 2023-10-20 20:49:11 UTC
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>
Comment 8 Phil Ringnalda 2023-10-20 20:49:13 UTC
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>
Comment 9 Phil Ringnalda 2023-10-20 20:52:16 UTC
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."
Comment 10 Martin Renvoize 2023-10-23 09:56:14 UTC
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>
Comment 11 Nick Clemens (kidclamp) 2023-10-23 12:19:29 UTC
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>
Comment 12 Nick Clemens (kidclamp) 2023-10-23 12:19:32 UTC
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>
Comment 13 Nick Clemens (kidclamp) 2023-10-23 12:19:34 UTC
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>
Comment 14 Tomás Cohen Arazi 2023-10-25 14:10:08 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 15 Fridolin Somers 2023-10-26 08:22:45 UTC
Depends on Bug 35099 not in 23.05.x
Comment 16 Tomás Cohen Arazi 2023-10-26 19:37:41 UTC
This seems to have broken some selenium tests. Please fix ASAP.

https://jenkins.koha-community.org/job/Koha_Master/lastCompletedBuild/testReport/
Comment 17 Martin Renvoize 2023-10-27 14:47:44 UTC
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.
Comment 18 Martin Renvoize 2023-10-27 14:50:57 UTC
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.
Comment 19 Tomás Cohen Arazi 2023-10-27 19:43:31 UTC
Follow-up pushed to master. Thanks!