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

Implementation of new notification system #254

Merged
merged 11 commits into from
Jan 4, 2024
21 changes: 17 additions & 4 deletions readthedocsext/theme/templates/builds/build_detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@
</div>
</div>

{# TODO: remove this `build.output block completely #}
{% if build.output %}
{# If we have build output, this is an old build #}
<p>
Expand Down Expand Up @@ -187,11 +188,23 @@ <h3>{% trans "Build Errors" %}</h3>
{% comment %}
Error list
{% endcomment %}
<div class="ui attached inverted red segment" data-bind="visible: error" style="display: none;">
<i class="fa-solid fa-exclamation circular icon"></i>
<span data-bind="text: error"></span>
<span>{% trans "For more information on this error, see our documentation." %}
{% if build.notifications.exists %}
{# Show error notifications as "build-error" #}
humitos marked this conversation as resolved.
Show resolved Hide resolved
{% for notification in build.notifications.all %}
{% if notification.get_message.type == "error" %}
<div class="ui attached inverted red segment">
<i class="fa-solid fa-exclamation circular icon"></i>
<span>{{ notification.get_message.get_rendered_body|safe }}</span>
humitos marked this conversation as resolved.
Show resolved Hide resolved
<span>{% trans "For more information on this error, see our documentation." %}</span>
</div>
{% endif %}
{% endfor %}
humitos marked this conversation as resolved.
Show resolved Hide resolved
{% else %}
<div class="ui attached inverted red segment" data-bind="visible: error" style="display: none;">
<span data-bind="text: error"></span>
<span>{% trans "For more information on this error, see our documentation." %}</span>
</div>
{% endif %}

<div class="ui inverted bottom attached segment transition slide">

Expand Down
36 changes: 13 additions & 23 deletions readthedocsext/theme/templates/includes/utils/messages.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,23 @@

{% comment %}
This template is used by the base template to provide a list of messages on
the top of the page. If the message has a pk it is a persisent or sticky
message level. Otherwise, Django default level notifications are treated as
the top of the page. Django default level notifications are treated as
temporary messages, using SUI toast messages instead of block elements in the
page content pane.
{% endcomment %}

{% if messages %}
{% for message in messages %}
{# Message is sticky/persistent, show these like normal. #}
{% if message.pk %}
<div class="ui message {{ message.tags }}"
data-bind="message: {dismiss_url: '{% url "message_mark_read" message.pk %}'}">
<i class="fa-duotone fa-circle-xmark close icon"></i>
{{ message }}
</div>
{% else %}
{% comment %}
This is a little bit of a hack, as toasts are purely JS and don't
require an underlying element. Django messages uses "tags" to specify
info/success/error classes on the messages, which just happen to align
with SUI classes too.
{% endcomment %}
{# fmt:off #}
{% spaceless %}
<div style="display: none;"
data-bind="
{% comment %}
This is a little bit of a hack, as toasts are purely JS and don't
require an underlying element. Django messages uses "tags" to specify
info/success/error classes on the messages, which just happen to align
with SUI classes too.
{% endcomment %}
{# fmt:off #}
{% spaceless %}
<div style="display: none;"
data-bind="
semanticui: {
toast: {
message: '{{ message|escapejs }}',
Expand All @@ -38,8 +29,7 @@
}
}">
</div>
{% endspaceless %}
{# fmt:on #}
{% endif %}
{% endspaceless %}
{# fmt:on #}
{% endfor %}
{% endif %}
15 changes: 15 additions & 0 deletions src/js/build/detail.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,8 @@ export class BuildDetailView {
/** @observable {Boolean} There was doc output in the build */
this.can_view_docs = ko.observable(false);

this.poll_api_counts = 0;

// Consolidate all of the observable updates that depend on build state
this.state.subscribe((state) => {
this.update_state(state);
Expand Down Expand Up @@ -413,6 +415,9 @@ export class BuildDetailView {
this.state(data.state);
this.state_display(data.state_display);

this.poll_api_counts = this.poll_api_counts + 1;


// This is a mock command used to preview the command output.
// TODO probably do this in the application instead
this.add_command({
Expand All @@ -438,6 +443,16 @@ export class BuildDetailView {
// this update happens at the very end of API updates instead.
if (this.is_finished()) {
this.is_polling(false);

// HACK: this is a small hack to avoid implementing the new notification system
// on ext-theme at this point. This will come in a future PR.
// The notifications are rendered properly via Django template in a static way.
// So, we refresh the page once the build has finished to make Django render the notifications.
// We use a check of 1 API poll for those builds that are already finished when opened.
// The new dashboard will implement the new notification system in a nicer way using APIv3.
if (this.poll_api_counts !== 1) {
location.reload();
}
} else {
setTimeout(() => {
this.poll_api();
Expand Down