Bug 35285 - Centralise notice content wrapping for HTML output
Summary: Centralise notice content wrapping for HTML output
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial
Assignee: Martin Renvoize (ashimema)
QA Contact: Testopia
URL:
Keywords:
Depends on: 30287 34704
Blocks: 30723 33260 35286 35459 36808 36809
  Show dependency treegraph
 
Reported: 2023-11-08 09:44 UTC by Martin Renvoize (ashimema)
Modified: 2024-05-31 13:05 UTC (History)
5 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:
24.05.00,23.11.06
Circulation function:


Attachments
Bug 35285: Add non-html template support to html_content wrapping (2.17 KB, patch)
2023-11-08 10:40 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35285: Remove switch in notices.tt (2.18 KB, patch)
2023-11-08 10:46 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35285: Add non-html template support to html_content wrapping (2.39 KB, patch)
2023-11-13 16:48 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35285: Remove switch in notices.tt (2.18 KB, patch)
2023-11-13 16:48 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35285: Unit tests (4.43 KB, patch)
2023-11-13 16:48 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35285: Add non-html template support to html_content wrapping (2.43 KB, patch)
2023-11-15 09:16 UTC, David Nind
Details | Diff | Splinter Review
Bug 35285: Remove switch in notices.tt (2.23 KB, patch)
2023-11-15 09:16 UTC, David Nind
Details | Diff | Splinter Review
Bug 35285: Unit tests (4.48 KB, patch)
2023-11-15 09:16 UTC, David Nind
Details | Diff | Splinter Review
Bug 35285: Add non-html template support to html_content wrapping (2.43 KB, patch)
2024-05-07 18:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35285: Remove switch in notices.tt (2.23 KB, patch)
2024-05-07 18:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35285: Unit tests (4.48 KB, patch)
2024-05-07 18:25 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35285: Add non-html template support to html_content wrapping (2.43 KB, patch)
2024-05-07 18:30 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35285: Remove switch in notices.tt (2.23 KB, patch)
2024-05-07 18:30 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35285: Unit tests (4.48 KB, patch)
2024-05-07 18:30 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35285: Add non-html template support to html_content wrapping (2.43 KB, patch)
2024-05-08 06:03 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35285: Remove switch in notices.tt (2.23 KB, patch)
2024-05-08 06:03 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35285: Unit tests (4.48 KB, patch)
2024-05-08 06:03 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35285: Add non-html template support to html_content wrapping (2.42 KB, patch)
2024-05-08 08:27 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35285: Remove switch in notices.tt (2.23 KB, patch)
2024-05-08 08:27 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35285: Unit tests (4.46 KB, patch)
2024-05-08 08:27 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35285: Add non-html template support to html_content wrapping (2.48 KB, patch)
2024-05-14 18:05 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 35285: Remove switch in notices.tt (2.28 KB, patch)
2024-05-14 18:05 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 35285: Unit tests (4.52 KB, patch)
2024-05-14 18:05 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 35285: (QA follow-up) Tidy (963 bytes, patch)
2024-05-14 18:05 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize (ashimema) 2023-11-08 09:44:01 UTC
Whilst working through some notice bugs I noted that we now have a few different places where we wrap the content for output in various instants.

I think we could centralise on the html_content method I introduce in bug 30287 and add support for the 'pre' (or perhaps replace pre with <div style="white-space: pre-wrap;"> as suggested in https://stackoverflow.com/questions/248011/how-do-i-wrap-text-in-a-pre-tag) for non html encoded templates.

Thoughts?
Comment 1 Martin Renvoize (ashimema) 2023-11-08 10:40:16 UTC
Created attachment 158657 [details] [review]
Bug 35285: Add non-html template support to html_content wrapping

This patch adds support for messages generated using non-html formatted
notice templates to the html_content method of Koha::Notice::Message.

We continue to wrap content for html generated messages with the
appropriate headers, css and title.

For non-html generated content we wrap in the <div style="white-space:
pre-wrap"> block to maintain text formatting as defined in the original
plaintext template.
Comment 2 Martin Renvoize (ashimema) 2023-11-08 10:40:57 UTC
Work in progress
Comment 3 Martin Renvoize (ashimema) 2023-11-08 10:46:23 UTC
Created attachment 158658 [details] [review]
Bug 35285: Remove switch in notices.tt

We now wrap appropriately for non-html formatted messages as
part of the html_content method. This means we can remove the
case from members/notices.tt and rely on html_content doing
the right thing for notice previews.
Comment 4 Martin Renvoize (ashimema) 2023-11-13 16:48:35 UTC
Created attachment 158889 [details] [review]
Bug 35285: Add non-html template support to html_content wrapping

This patch adds support for messages generated using non-html formatted
notice templates to the html_content method of Koha::Notice::Message.

We continue to wrap content for html generated messages with the
appropriate headers, css and title.

For non-html generated content we wrap in the <div style="white-space:
pre-wrap"> block to maintain text formatting as defined in the original
plaintext template.

Test
Follow the test plan for bug 30287, nothing should outwardly change.
Comment 5 Martin Renvoize (ashimema) 2023-11-13 16:48:37 UTC
Created attachment 158890 [details] [review]
Bug 35285: Remove switch in notices.tt

We now wrap appropriately for non-html formatted messages as
part of the html_content method. This means we can remove the
case from members/notices.tt and rely on html_content doing
the right thing for notice previews.
Comment 6 Martin Renvoize (ashimema) 2023-11-13 16:48:40 UTC
Created attachment 158891 [details] [review]
Bug 35285: Unit tests

This patch adds unit tests for the new 'is_html' function introduced in
Koha::Notice::Message and update the tests to include the new plaintext
handling of html_content.
Comment 7 David Nind 2023-11-15 09:16:21 UTC
Created attachment 158960 [details] [review]
Bug 35285: Add non-html template support to html_content wrapping

This patch adds support for messages generated using non-html formatted
notice templates to the html_content method of Koha::Notice::Message.

We continue to wrap content for html generated messages with the
appropriate headers, css and title.

For non-html generated content we wrap in the <div style="white-space:
pre-wrap"> block to maintain text formatting as defined in the original
plaintext template.

Test
Follow the test plan for bug 30287, nothing should outwardly change.

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 David Nind 2023-11-15 09:16:24 UTC
Created attachment 158961 [details] [review]
Bug 35285: Remove switch in notices.tt

We now wrap appropriately for non-html formatted messages as
part of the html_content method. This means we can remove the
case from members/notices.tt and rely on html_content doing
the right thing for notice previews.

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 David Nind 2023-11-15 09:16:27 UTC
Created attachment 158962 [details] [review]
Bug 35285: Unit tests

This patch adds unit tests for the new 'is_html' function introduced in
Koha::Notice::Message and update the tests to include the new plaintext
handling of html_content.

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 Martin Renvoize (ashimema) 2024-05-07 18:25:52 UTC Comment hidden (obsolete)
Comment 11 Martin Renvoize (ashimema) 2024-05-07 18:25:55 UTC Comment hidden (obsolete)
Comment 12 Martin Renvoize (ashimema) 2024-05-07 18:25:58 UTC Comment hidden (obsolete)
Comment 13 Martin Renvoize (ashimema) 2024-05-07 18:26:17 UTC
Just a quick rebase
Comment 14 Martin Renvoize (ashimema) 2024-05-07 18:30:51 UTC Comment hidden (obsolete)
Comment 15 Martin Renvoize (ashimema) 2024-05-07 18:30:53 UTC Comment hidden (obsolete)
Comment 16 Martin Renvoize (ashimema) 2024-05-07 18:30:57 UTC Comment hidden (obsolete)
Comment 17 Martin Renvoize (ashimema) 2024-05-08 06:03:41 UTC Comment hidden (obsolete)
Comment 18 Martin Renvoize (ashimema) 2024-05-08 06:03:44 UTC Comment hidden (obsolete)
Comment 19 Martin Renvoize (ashimema) 2024-05-08 06:03:46 UTC Comment hidden (obsolete)
Comment 20 Martin Renvoize (ashimema) 2024-05-08 08:27:35 UTC Comment hidden (obsolete)
Comment 21 Martin Renvoize (ashimema) 2024-05-08 08:27:38 UTC Comment hidden (obsolete)
Comment 22 Martin Renvoize (ashimema) 2024-05-08 08:27:41 UTC Comment hidden (obsolete)
Comment 23 Nick Clemens (kidclamp) 2024-05-14 18:05:40 UTC
Created attachment 166719 [details] [review]
Bug 35285: Add non-html template support to html_content wrapping

This patch adds support for messages generated using non-html formatted
notice templates to the html_content method of Koha::Notice::Message.

We continue to wrap content for html generated messages with the
appropriate headers, css and title.

For non-html generated content we wrap in the <div style="white-space:
pre-wrap"> block to maintain text formatting as defined in the original
plaintext template.

Test
Follow the test plan for bug 30287, nothing should outwardly change.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 24 Nick Clemens (kidclamp) 2024-05-14 18:05:42 UTC
Created attachment 166720 [details] [review]
Bug 35285: Remove switch in notices.tt

We now wrap appropriately for non-html formatted messages as
part of the html_content method. This means we can remove the
case from members/notices.tt and rely on html_content doing
the right thing for notice previews.

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 25 Nick Clemens (kidclamp) 2024-05-14 18:05:44 UTC
Created attachment 166721 [details] [review]
Bug 35285: Unit tests

This patch adds unit tests for the new 'is_html' function introduced in
Koha::Notice::Message and update the tests to include the new plaintext
handling of html_content.

Signed-off-by: David Nind <david@davidnind.com>

test

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 26 Nick Clemens (kidclamp) 2024-05-14 18:05:47 UTC
Created attachment 166722 [details] [review]
Bug 35285: (QA follow-up) Tidy

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 27 Katrin Fischer 2024-05-17 08:22:43 UTC
This one seems at least in a grey area for being an enh. I am going to push it as it's mostly affecting display in the staff interface right now.
Comment 28 Katrin Fischer 2024-05-17 08:37:16 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 29 Fridolin Somers 2024-05-29 13:18:34 UTC
Pushed to 23.11.x for 23.11.06
Comment 30 Lucas Gass (lukeg) 2024-05-31 13:05:31 UTC
This feels like an enhancement to me. I'm not going to include this in 23.05.x.