Skip to content

Hook up global alerts for New Homepage using current sitewide alert system #209

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.

Sign up for GitHub

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

Merged
djanelle-mit merged 3 commits into from
Aug 12, 2026
+52 −15
Merged

Hook up global alerts for New Homepage using current sitewide alert system #209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4ad9fb7
Hooking up sitewide alert system to use global alerts style and marku…
djanelle-mit Aug 12, 2026
8d1f874
Small size update to alert icon
djanelle-mit Aug 12, 2026
5ffb8b3
Increment style sheet version
djanelle-mit Aug 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion web/app/themes/mitlib-parent/css/v2/components/alerts.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
#global-alert {
position: relative;

&.hidden {display: none;}

.content-wrapper {
gap: var(--sp-400);
}
Expand All @@ -31,7 +33,8 @@
flex-grow: 1;

i {
margin-top: var(--sp-100);
font-size: 18px;
margin-top: 3px;
flex-shrink: 0;
flex-grow: 0;
}
Expand Down
60 changes: 47 additions & 13 deletions web/app/themes/mitlib-parent/js/alerts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Check to see if the current page is a v2 page. If it is, we'll use v2 alert markup and load a v2 close button in the respective functions. If not, we'll use v1.
var isV2page = document.body.classList.contains('v2-page');

@matt-bernhardt matt-bernhardt Aug 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Non-blocking comment:
This is probably a good candidate for a constant, rather than a variable (although a variable that is known to be a unique name like this is also probably fine.

The question I'm grappling with is whether a read-only javascript constant like this might be useful in other contexts, for any other javascript-based feature that will need to be operational through the transition between v1 and v2 pages. The first candidates that come to mind are the map and the calendar, but those are each only on a single page - so any issues we run into with those features can be bundled into the migration of those templates.

The various hours widgets will need to run simultaneously on v1 and v2 pages, so depending on whether those widgets change significantly that's a candidate.

For the moment, this is likely fine - but as we get deeper into the transition this line might be something that we should move to functions.php and to spit out a constant as in https://github.com/MITLibraries/mitlib-wp-network/blob/master/web/app/themes/mitlib-parent/functions.php#L238 .

@djanelle-mit djanelle-mit Aug 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ahh interesting... Maybe we couple moving this to a constant with whatever the next feature is that needs to check for this condition. I haven't seen that "create a JS const via PHP" mechanism before!


// Loads alert-level posts on the top of all pages
function filterAlerts(posts) {
// This processes an array of posts for valid, confirmed, alerts
Expand Down Expand Up @@ -56,12 +59,26 @@ function renderAlert(markup,id) {
}

function setClosable(alert_ID) {
// Add a Close icon/svg/button
$('.posts--preview--alerts .post').append('<a href="#0" id="close" class="action-close"><span class="sr">Dismiss</span><i class="fa fa-circle-xmark" aria-hidden="true"></i></a>');
// On click
// Add a Close button. We use v2 markup only for pages that are v2 pages. Otherwise, we'll use v1 markup.
if(isV2page) {
// v2 close button markup
$('#global-alert').append('<a id="close" href="#" class="dismiss"><i class="fa-sharp fa-light fa-xmark"></i></a>');
} else {
// v1 close button markup
$('.posts--preview--alerts .post').append('<a href="#0" id="close" class="action-close"><span class="sr">Dismiss</span><i class="fa fa-circle-xmark" aria-hidden="true"></i></a>');
}

// On click of the link with the close id
$('#close').click(function(){
// Add the necessary transition hide class
$(this).closest('.posts--preview--alerts').addClass('transition-vertical--hide');

// Apply relevant logic based on v1 or v2 alert. Add relevant class to hide the alert.
if(isV2page) {
// v2
$(this).closest('#global-alert').addClass('hidden');
} else {
// v1
$(this).closest('.posts--preview--alerts').addClass('transition-vertical--hide');
}
// Bump the hours calendar down, if it is present.
moveCalendar(-152);

@matt-bernhardt matt-bernhardt Aug 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Non-blocking comment:
I'm realizing as I see the alert you created that having a hard-coded value like this isn't particularly robust to different content lengths. I'm not sure what a better option would be offhand, and I'm not asking that we delay this PR while dealing with an existing issue like this, though.

This is also something to consider, as the hours page migrates, whether we need separate setClosable functions that are dedicated to v1 and v2. At the moment with two branches in the same function, the complexity of this function is probably at the limit of what I'd want to support long term. Adding a third branch would push me to recommending we split the call to setClosable on line 143 into something like:

		// If this is a closable alert
		if (true === alert_posts_arr[i].meta.closable) {
            if (isV2page) {
			    setClosableV2(alert_ID);
            } else {
			    setClosableV1(alert_ID);
            }
		}

(This introduces a separate maintenance concern as we'd be maintaining two functions in parallel, but I'd rather that than a single function that branches more than twice - or maybe there's another more efficient mechanism somehow, if we prototype a bit more)

// If localStorage
Expand Down Expand Up @@ -92,15 +109,32 @@ function showAlerts(json) {
// Check for empty title
alert_title = ('' === alert_posts_arr[i].title.rendered) ? 'Alert!' : alert_posts_arr[i].title.rendered;

// Alert HTML template
alert_template = '<div class="posts--preview--alerts transition-vertical transition-vertical--hide">' +
'<div class="post alert--critical flex-container">' +
'<i class="fa fa-circle-exclamation" aria-hidden="true"></i>' +
'<div class="content-post alertText">' +
'<h3>' + alert_title + '</h3> ' + alert_posts_arr[i].content.rendered +
// If this is a v2 page, use the v2 alert markup. Otherwise, use the v1 markup.
if (isV2page) {
// Alert HTML template (v2)
alert_template = '<section id="global-alert">' +

@matt-bernhardt matt-bernhardt Aug 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Comment:
As you've noted, this is invalid markup if there are ever more than one alert at a time - but this is outside of our anticipated use so I'm not sure that this is something I'm going to ask be changed here.

I do suspect that the use of .closest will end up helping, but ultimately not using repeated id values is the better practice.

@djanelle-mit djanelle-mit Aug 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'm also realizing as I look at the code here again, that we already have a duplicate ID problem since for for both v1 and v2 alerts we're adding a link with #close to trigger the dismiss.

This doesn't happen very often, but when I tested multiple alerts the .closest seemed to make the dismiss buttons work on the correct alert (and didn't fire on all of them) so I think we're OK here.

I'm going to leave this as a future-us thing we fix with whatever longer term solution we come to for Alerts.

'<div class="content-wrapper">' +
'<div class="global-alert-message">' +
'<i class="fa-solid fa-triangle-exclamation"></i>' +
'<header>' +
'<h2>' + alert_title + '</h2>' +
'<p>' + alert_posts_arr[i].content.rendered + '</p>' +
'</header>' +
'</div>' +
'</div>'
'</section>';

} else {
// Alert HTML template (v1)
alert_template = '<div class="posts--preview--alerts transition-vertical transition-vertical--hide">' +
'<div class="post alert--critical flex-container">' +
'<i class="fa fa-circle-exclamation" aria-hidden="true"></i>' +
'<div class="content-post alertText">' +
'<h3>' + alert_title + '</h3> ' + alert_posts_arr[i].content.rendered +
'</div>' +
'</div>' +
'</div>' +
'</div>';
'</div>';
}

renderAlert(alert_template,alert_ID);

Expand Down
2 changes: 1 addition & 1 deletion web/app/themes/mitlib-parent/style.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
Theme Name: MITlib Parent
Author: MIT Libraries
Version: 0.12
Version: 0.13
Description: The parent theme for the MIT Libraries' Pentagram-designed identity.

*/
Expand Down