During review of bug 33487 I made the suggestion that we should have more CSS options for our notices at the system preference level. I've also hit this when looking into bug 30287. So.. recording this here and hopefully we'll find some time to work on it. Personally.. I'd have * EmailNoticeCSS - CSS stored in the DB for email notice CSS * EmailNoticeCSSHosted - Location of CSS file for email notice CSS (to retain the old NoticeCSS option) * PrintNoticeCSS - CSS stored in the DB for print notice CSS * PrintNoticeCSSHosted - Location of the CSS file for the print notice CSS (to reatin the old SlipCSS option) SMS doesn't support HTML/CSS so no need for that transport type to have these CSS options.
Correction, it was bug 33478
This would be easier to implement if we had continued down the path from bug 31714.. we'd have fewer places to change :(
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33478#c78 is the original comment regarding this.
Created attachment 172414 [details] [review] Bug 35267: Add and Update Notice CSS system preferences This patch clarifies the system level notice css preferences. We rename NoticeCSS to EmailNoticeStylesheet We rename SlipCSS to PrintSlipStylesheet We add AllNoticeStylesheet We add AllNoticeCSS to contain inline CSS for all html notices We add EmailNoticeCSS to contain inline CSS for Email notices We add PrintNoticeCSS to contain inline CSS for Print notices We add PrintSlipCSS to contain inline CSS for Print slips
Created attachment 172415 [details] [review] Bug 35267: Add preferences to UI
Created attachment 172416 [details] [review] Bug 35267: Apply preferences to preview and mailing This patch applies the new style preferences to the preview display and email sending code. TODO: We still need to apply these more clearly to the printing code, both batch and slip printing
Created attachment 172417 [details] [review] Bug 35267: Remove printslip css from printnotice This is the begginings of trying to apply styling selectively in the various printing scripts for Koha's notices and slips.
Created attachment 172418 [details] [review] Bug 35267: Expose style preferences in template editor
There's still more work to do here to make sure we catch all the different printing cases.. but I think this is a solid start. I'd love some help/thoughts on the notice editing UI changes and the relocation of preferences to one place under the 'Tools' section for system preferences.
Created attachment 172457 [details] [review] Bug 35267: Add and Update Notice CSS system preferences This patch clarifies the system level notice css preferences. We rename NoticeCSS to EmailNoticeStylesheet We rename SlipCSS to PrintSlipStylesheet We add AllNoticeStylesheet We add AllNoticeCSS to contain inline CSS for all html notices We add EmailNoticeCSS to contain inline CSS for Email notices We add PrintNoticeCSS to contain inline CSS for Print notices We add PrintSlipCSS to contain inline CSS for Print slips
Created attachment 172458 [details] [review] Bug 35267: Add preferences to UI
Created attachment 172459 [details] [review] Bug 35267: Expose style preferences in template editor
Created attachment 172460 [details] [review] Bug 35267: Apply preferences to preview and mailing This patch applies the new style preferences to the preview display and email sending code.
Created attachment 172461 [details] [review] Bug 35267: Move JS to bottom in printslip
Created attachment 172462 [details] [review] Bug 35267: Migrate accounts_summary to printslip This patch migrates the 'slip' printing for the ACCOUNTS_SUMMARY slip to circ/printslip as this is a slip rather than a notice.
Created attachment 172463 [details] [review] Bug 35267: Migrate notice printing to members/print_notice This migrates the bulk notice printing from tools/print_notice.pl using circ/printslip.tt to members/print_notice.pl using members/print_notice.tt This clarified that we are working on pre-existing notices in the message queue and as such should be applying relevant print/email notice styling and not slip styling.
Created attachment 172474 [details] [review] Bug 35267: Add and Update Notice CSS system preferences This patch clarifies the system level notice css preferences. We rename NoticeCSS to EmailNoticeStylesheet We rename SlipCSS to PrintSlipStylesheet We add AllNoticeStylesheet We add AllNoticeCSS to contain inline CSS for all html notices We add EmailNoticeCSS to contain inline CSS for Email notices We add PrintNoticeCSS to contain inline CSS for Print notices We add PrintSlipCSS to contain inline CSS for Print slips
Created attachment 172475 [details] [review] Bug 35267: Add preferences to UI
Created attachment 172476 [details] [review] Bug 35267: Expose style preferences in template editor
Created attachment 172477 [details] [review] Bug 35267: Apply preferences to preview and mailing This patch applies the new style preferences to the preview display and email sending code.
Created attachment 172478 [details] [review] Bug 35267: Move JS to bottom in printslip
Created attachment 172479 [details] [review] Bug 35267: Migrate accounts_summary to printslip This patch migrates the 'slip' printing for the ACCOUNTS_SUMMARY slip to circ/printslip as this is a slip rather than a notice.
Created attachment 172480 [details] [review] Bug 35267: Migrate notice printing to members/print_notice This migrates the bulk notice printing from tools/print_notice.pl using circ/printslip.tt to members/print_notice.pl using members/print_notice.tt This clarified that we are working on pre-existing notices in the message queue and as such should be applying relevant print/email notice styling and not slip styling.
# Test plan This is going to be a fun road to travel. ## UI 1) Apply patches and run database updates 2) Note that 'NoticeCSS' and 'SlipCSS' have been renamed to 'EmailNoticeStylesheet' and 'PrintSlipStylesheet' to better reflect their content and both have been migrated into 'Admin > System preferences > Tools > Notices and slips' 3) Note in the same location in system preferences, we now also have 'EmailNoticeCSS', 'PrintNoticeCSS' and 'PrintSlipCSS' preferences that allow direct entry of CSS rules for their respective notice message transports. 4) Under 'Tools > Notices and Slips', edit a notice and note we have a new accordion near the top of the page that displays the content of 'AllNoticeCSS', 'EmailNoticeCSS', 'PrintNoticeCSS' and 'PrintSlipCSS' preferences for the end users reference 4a) If the user has administrative permissions, they should also see an 'Edit' button that's a shortcut the the relevant preference in the system preferences ## Adding styles 1) Add a simple style for testing to 'AllNoticeCSS' `.alltest { color: red }` 2) Add a simple style for testing to 'EmailNoticeCSS' `.emailtest { color: green }` 3) Add a simple style for testing to 'PrintNoticeCSS' `.printtest { color: organge }` 4) Let use the WELCOME notice for testing.. add a simple html template as follows. ``` <p> This is a notice for testing</br> <span class="alltest">All message transport types should display this red</span><br> <span class="emailtest">Only email notices should display this green</span></br> <span class="printtest">Only print notices should display this orange</span><br> All done </p> ``` 5) Lets use 'Henry A' to test the notice 5a) Do NOT set an email address for Henry 5b) Queue a 'Welcome' print notice using 'More > Send welcome' 5c) Add a primary email address for Henry 5d) Queue a 'Welcome' email notice using 'More > Send welcome' 6) Navigate to the 'Notices' tab for Henry 7) Click the title of the print notice to display a preview and note the correct colors are displayed 8) Click the title of the email notice to display a preview and note the correct colors are displayed ## Testing the functionality 1) Run 'gather_print_notices.pl' with options to output to html file 2) Confirm that the generated notice contains the correct classes and styles 3) Enable 'ManageNotices' preference from the dependency bug 4) Navigate to 'Tools > Mange notices' 5) Select both notices and print using the bottom button 6) Confirm that the two notices display the different colors and are on two separate pages in the print preview.
Created attachment 172485 [details] [review] Bug 35267: Add and Update Notice CSS system preferences This patch clarifies the system level notice css preferences. We rename NoticeCSS to EmailNoticeStylesheet We rename SlipCSS to PrintSlipStylesheet We add AllNoticeStylesheet We add AllNoticeCSS to contain inline CSS for all html notices We add EmailNoticeCSS to contain inline CSS for Email notices We add PrintNoticeCSS to contain inline CSS for Print notices We add PrintSlipCSS to contain inline CSS for Print slips
Created attachment 172486 [details] [review] Bug 35267: Add preferences to UI
Created attachment 172487 [details] [review] Bug 35267: Expose style preferences in template editor
Created attachment 172488 [details] [review] Bug 35267: Apply preferences to preview and mailing This patch applies the new style preferences to the preview display and email sending code.
Created attachment 172489 [details] [review] Bug 35267: Move JS to bottom in printslip
Created attachment 172490 [details] [review] Bug 35267: Migrate accounts_summary to printslip This patch migrates the 'slip' printing for the ACCOUNTS_SUMMARY slip to circ/printslip as this is a slip rather than a notice.
Created attachment 172491 [details] [review] Bug 35267: Migrate notice printing to members/print_notice This migrates the bulk notice printing from tools/print_notice.pl using circ/printslip.tt to members/print_notice.pl using members/print_notice.tt This clarified that we are working on pre-existing notices in the message queue and as such should be applying relevant print/email notice styling and not slip styling.
This clarifies the original Notice and Slip CSS options expanding from just supporting linked stylesheets to using local style input fields too. We do not yet handle clarifying how the per notice per transport type level CSS can be defined. This needs some thought and I've got that as a follow-up bug 38090
This also, does not fully implement the stylesheet link options for the multi-print handling code.. that was already failing in bug 33260 and will also need some thought.. I think this should also be a follow-up bug as I'm really not sure if anyone is actually using that functionality in real life.
Created attachment 175531 [details] [review] Bug 35267: Add and Update Notice CSS system preferences This patch clarifies the system level notice css preferences. We rename NoticeCSS to EmailNoticeStylesheet We rename SlipCSS to PrintSlipStylesheet We add AllNoticeStylesheet We add AllNoticeCSS to contain inline CSS for all html notices We add EmailNoticeCSS to contain inline CSS for Email notices We add PrintNoticeCSS to contain inline CSS for Print notices We add PrintSlipCSS to contain inline CSS for Print slips
Created attachment 175532 [details] [review] Bug 35267: Add preferences to UI
Created attachment 175533 [details] [review] Bug 35267: Expose style preferences in template editor
Created attachment 175534 [details] [review] Bug 35267: Apply preferences to preview and mailing This patch applies the new style preferences to the preview display and email sending code.
Created attachment 175535 [details] [review] Bug 35267: Move JS to bottom in printslip
Created attachment 175536 [details] [review] Bug 35267: Migrate accounts_summary to printslip This patch migrates the 'slip' printing for the ACCOUNTS_SUMMARY slip to circ/printslip as this is a slip rather than a notice.
Bugs reordered and rebased so we don't hold up this important fix/enhancement behind the more contentious notices management page and print options bug.
With the reorder the following parts of the above test plan should be ignored.. or rather only tested when the relevant bug is applied atop this one: 3) Enable 'ManageNotices' preference from the dependency bug 4) Navigate to 'Tools > Mange notices' 5) Select both notices and print using the bottom button 6) Confirm that the two notices display the different colors and are on two separate pages in the print preview.
After applying the patches, there was an error with the databae update updatedatabase String found where operator expected at /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_35267.pl line 20, near "$out "Added new system preference 'AllNoticeCSS'"" (Missing operator before "Added new system preference 'AllNoticeCSS'"?) DEV atomic update /kohadevbox/koha/installer/data/mysql/atomicupdate/bug_35267.pl [23:07:10] ERROR: {UNKNOWN}: Can't use an undefined value as a subroutine reference at /kohadevbox/koha/C4/Installer.pm line 741. at /kohadevbox/koha/C4/Installer.pm line 743
Created attachment 175706 [details] [review] Bug 35267: Add and Update Notice CSS system preferences This patch clarifies the system level notice css preferences. We rename NoticeCSS to EmailNoticeStylesheet We rename SlipCSS to PrintSlipStylesheet We add AllNoticeStylesheet We add AllNoticeCSS to contain inline CSS for all html notices We add EmailNoticeCSS to contain inline CSS for Email notices We add PrintNoticeCSS to contain inline CSS for Print notices We add PrintSlipCSS to contain inline CSS for Print slips
Created attachment 175707 [details] [review] Bug 35267: Add preferences to UI
Created attachment 175708 [details] [review] Bug 35267: Expose style preferences in template editor
Created attachment 175709 [details] [review] Bug 35267: Apply preferences to preview and mailing This patch applies the new style preferences to the preview display and email sending code.
Created attachment 175710 [details] [review] Bug 35267: Move JS to bottom in printslip
Created attachment 175711 [details] [review] Bug 35267: Migrate accounts_summary to printslip This patch migrates the 'slip' printing for the ACCOUNTS_SUMMARY slip to circ/printslip as this is a slip rather than a notice.
Created attachment 175715 [details] [review] Bug 35267: Add and Update Notice CSS system preferences This patch clarifies the system level notice css preferences. We rename NoticeCSS to EmailNoticeStylesheet We rename SlipCSS to PrintSlipStylesheet We add AllNoticeStylesheet We add AllNoticeCSS to contain inline CSS for all html notices We add EmailNoticeCSS to contain inline CSS for Email notices We add PrintNoticeCSS to contain inline CSS for Print notices We add PrintSlipCSS to contain inline CSS for Print slips
Created attachment 175716 [details] [review] Bug 35267: Add preferences to UI
Created attachment 175717 [details] [review] Bug 35267: Expose style preferences in template editor
Created attachment 175718 [details] [review] Bug 35267: Apply preferences to preview and mailing This patch applies the new style preferences to the preview display and email sending code.
Created attachment 175719 [details] [review] Bug 35267: Move JS to bottom in printslip
Created attachment 175721 [details] [review] Bug 35267: Migrate accounts_summary to printslip This patch migrates the 'slip' printing for the ACCOUNTS_SUMMARY slip to circ/printslip as this is a slip rather than a notice.
Created attachment 175723 [details] [review] Bug 35267: Add and Update Notice CSS system preferences This patch clarifies the system level notice css preferences. We rename NoticeCSS to EmailNoticeStylesheet We rename SlipCSS to PrintSlipStylesheet We add AllNoticeStylesheet We add AllNoticeCSS to contain inline CSS for all html notices We add EmailNoticeCSS to contain inline CSS for Email notices We add PrintNoticeCSS to contain inline CSS for Print notices We add PrintSlipCSS to contain inline CSS for Print slips
Created attachment 175724 [details] [review] Bug 35267: Add preferences to UI
Created attachment 175725 [details] [review] Bug 35267: Expose style preferences in template editor
Created attachment 175726 [details] [review] Bug 35267: Apply preferences to preview and mailing This patch applies the new style preferences to the preview display and email sending code.
Created attachment 175727 [details] [review] Bug 35267: Move JS to bottom in printslip
Created attachment 175728 [details] [review] Bug 35267: Migrate accounts_summary to printslip This patch migrates the 'slip' printing for the ACCOUNTS_SUMMARY slip to circ/printslip as this is a slip rather than a notice.
Created attachment 175729 [details] [review] Bug 35267: Update unit tests This patch simply updates the existing unit tests to rename NoticeCSS to AllNoticesStylesheet as per the rest of the patchset.
Looks like I lost a colon during the patch re-arrangement, thanks for spotting it David :)