Skip to content

Add base for alert support#3300

Open
GuillaumeGomez wants to merge 1 commit intorust-lang:mainfrom
GuillaumeGomez:alert-support
Open

Add base for alert support#3300
GuillaumeGomez wants to merge 1 commit intorust-lang:mainfrom
GuillaumeGomez:alert-support

Conversation

@GuillaumeGomez
Copy link
Copy Markdown
Member

This is a POC to add support for alerts. For now, it always generates an alert so you can give it a try. Even without JS, you can hide the alert. If JS is enabled when closing the alert, it will store the information and not show this alert again from this point.

@GuillaumeGomez GuillaumeGomez requested a review from a team as a code owner April 16, 2026 23:35
@github-actions github-actions Bot added the S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed label Apr 16, 2026
});
*/

pub(crate) fn get_alert() -> Option<(&'static str, usize)> {
Copy link
Copy Markdown
Member

@syphar syphar Apr 19, 2026

Choose a reason for hiding this comment

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

fyi:
I prepared parts of #3274 so we can include the alerts in that caching pipeline. When #3274 is merged etc I'm happy to update this PR to do it.

View changes since the review

{{- crate::icons::IconCircleInfo.render_solid(false, false, "") -}}
<span class="alert-text">{{- alert|safe -}}</span>
</div>
<script type="text/javascript">{%- include "alert.js" -%}</script>
Copy link
Copy Markdown
Member

@syphar syphar Apr 19, 2026

Choose a reason for hiding this comment

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

we probably should expose alert.js similar to menu.js as separate ( cacheable ) asset:

<script async src="/-/static/menu.js?{{ build_slug }}"></script>

View changes since the review

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point.

Comment thread crates/bin/docs_rs_web/templates/header/topbar_end.html
Copy link
Copy Markdown
Member

@syphar syphar left a comment

Choose a reason for hiding this comment

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

this might also interfere with caching, what do you think?

If these alerts are rare, we could just invoke a full cdn purge each time we set one.

otherwise we'll have to find some way to load the alert as partial (differently cached)

View changes since this review

@GuillaumeGomez
Copy link
Copy Markdown
Member Author

For this part, you're the expert so whatever you think is better.

@syphar
Copy link
Copy Markdown
Member

syphar commented Apr 21, 2026

For this part, you're the expert so whatever you think is better.

a partial loaded via separate URL.

So we have a "static" part (script / html), which is always the same, can be cached as long as the page itself (forever).

And a "dynamic" part loaded from a separate URL (like the version lists),
which is either "no alerts" or the alert.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants