Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix HTML in site messages #5358

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chrismytton
Copy link
Member

@chrismytton chrismytton commented Feb 7, 2025

If we detect that a site message contains HTML then don't use the html_para filter to avoid adding extra
elements which mess up spacing.

For FD-5103

Before

image

After

image

If we detect that a site message contains HTML then don't use the
html_para filter to avoid adding extra <br> elements which mess up
spacing.
Copy link

codecov bot commented Feb 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.42%. Comparing base (61e7964) to head (c37e9fc).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5358   +/-   ##
=======================================
  Coverage   82.42%   82.42%           
=======================================
  Files         416      416           
  Lines       32925    32926    +1     
  Branches     5289     5289           
=======================================
+ Hits        27138    27140    +2     
+ Misses       4225     4224    -1     
  Partials     1562     1562           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chrismytton chrismytton requested a review from dracos February 7, 2025 13:59
Copy link
Member

@dracos dracos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well, only suggesting might be good to hide the logic within a filter, and then it can be used everywhere a site message could be displayed?

@@ -1432,6 +1432,7 @@ sub site_message {
my $ooh = $self->ooh_times($body);
$msg = $ooh_msg if $ooh->active;
}
$msg =~ s/\r\n/\n/g;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this bit for? To make the test simpler?

@@ -1,7 +1,11 @@
[% site_message = c.cobrand.site_message %]
[% IF site_message && !rendered_site_message %]
<div class="site-message">
[% IF site_message.match('<[a-z][\s\S]*>', 'i') %]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some other places this would be good - report/new/form_after_heading.html for the reporting message, base/peterborough waste/header.html for waste, and templates/web/bathnes/around/_postcode_form_post.html. Perhaps a new Filter in Template.pm that does this check and then calls html_paragraph or not, so it's hidden from the templates, could just have site_message | html_para_if_no_html or something like that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants