-
Notifications
You must be signed in to change notification settings - Fork 53
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
enable using announcements to add content to alerts #1842
base: master
Are you sure you want to change the base?
Conversation
ee57a75
to
8b8b48e
Compare
8b8b48e
to
9c9dbcf
Compare
If an announcement has a location of 'alerts' then include that at the top of all alerts. The HTML version either uses "Read more" or the `button_text` property for the link. Announcements are added at the top of the alerts. Part of #1825
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we've manually done this, we've put the HTML p in the header under the linked logo, before the h1 - do we want it there, or under the h1 as is done here?
scripts/alertmailer.php
Outdated
if (property_exists($banner, 'button_text')) { | ||
$link_text = $banner->button_text; | ||
} | ||
$html_banner = '<strong>' . $banner->title . "</strong><p>" . $banner->content . '</p><p><a href="' . $banner->url . '">' . $link_text . '</a></p>'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we only output the second p if link_text isn't blank? People could put their own links inside the content presumably, and then not need a "Read more"? e.g. the last time we did this manually, the added text was:
<p style="font-size: 16px;">
The UK Parliament is back – read our blog post on
<a href="https://www.mysociety.org/2024/07/04/theyworkforyou-understanding-the-new-parliament/">resources to help you understand the new Parliament</a>,
and if you enjoy your alerts,
<a href="https://www.theyworkforyou.com/support-us/">donate to support our work</a>.
</p>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, thinking about it, as banner->content is same in both text and HTML, we couldn't really put HTML in it anyway, unless we had two different contents 🤷
If an announcement has a location of 'alerts' then include that at the top of all alerts. The HTML version either uses "Read more" or the
button_text
property for the link.Announcements are added at the top of the alerts.
Part of #1825