Skip to content

Commit

Permalink
Add API notification support to new dashboard (#300)
Browse files Browse the repository at this point in the history
* Proof of concept Lit notification element

* More experimenting with API notifications in other spots in the UI

* WIP, includes web component for notification

* Add notification message styles and overrides, mostly working implementation

This shows top level and build detail notifications with only some small
bugs remaining.

* Clean up rough edges on build view, JS, and templates

- Adds build error handling in templates
- Fixes polling bugs with build and notification API

* Relint files

* Add missing file

* Fix notification over polling

* Change API notification query to state__in

* Fix up docs

* Drop extra messages block, it is unused.

* Add missing web components API doc file

---------

Co-authored-by: Manuel Kaufmann <[email protected]>
  • Loading branch information
agjohnson and humitos authored Mar 26, 2024
1 parent 8eafe7f commit 0f36e2e
Show file tree
Hide file tree
Showing 27 changed files with 658 additions and 234 deletions.
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ commands:
steps:
- checkout
- run: pip install djhtml
- run: djhtml --tabwidth 2 readthedocsext/
- run: djhtml --check --tabwidth 2 readthedocsext/
run-build:
description: "Ensure built assets are up to date"
steps:
Expand Down
1 change: 1 addition & 0 deletions docs/api/javascript.rst
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ JavaScript API

js/application
js/plugins
js/webcomponents
js/views
4 changes: 2 additions & 2 deletions docs/api/js/plugins.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ Display

.. autofunction:: ./application/plugins.semanticui

.. autofunction:: ./application/plugins.webcomponent

Data
~~~~

Expand All @@ -25,8 +27,6 @@ How to pass data between the templates and our JavaScript in a secure manner.
Modules
~~~~~~~

.. autofunction:: ./application/plugins.message

.. autofunction:: ./application/plugins.chart

jQuery
Expand Down
14 changes: 14 additions & 0 deletions docs/api/js/webcomponents.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
Web components
==============

Elements
--------

.. js:autoclass:: NotificationListElement
:members:

.. js:autoclass:: NotificationElement
:members:

Views
-----
114 changes: 62 additions & 52 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@
"knockout": "^3.5.1",
"lato-font": "^3.0.0",
"less": "^4.2.0",
"less-loader": "^11.1.3",
"less-loader": "^12.2.0",
"lit": "^3.1.2",
"matchdep": "~2.0.0",
"mini-css-extract-plugin": "^2.7.6",
"plausible-tracker": "^0.3.8",
Expand Down

Large diffs are not rendered by default.

Large diffs are not rendered by default.

90 changes: 69 additions & 21 deletions readthedocsext/theme/static/readthedocsext/theme/js/vendor.js

Large diffs are not rendered by default.

16 changes: 8 additions & 8 deletions readthedocsext/theme/templates/account/login.html
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@

<div
{# The password URL is listed as an additional tab if present #}
class="ui small stackable secondary item menu {% if project_password_url %}three{% else %}two{% endif %}"
data-bind="semanticui: {tabs: {history: true, autoTabActivation: false}}">
class="ui small stackable secondary item menu {% if project_password_url %}three{% else %}two{% endif %}"
data-bind="semanticui: {tabs: {history: true, autoTabActivation: false}}">

{# This is purposely minimal, as the verbiage for "Log in with connected account" is a mountful #}
<a class="item" data-tab="vcs">
Expand All @@ -40,18 +40,18 @@
<a class="item {% if allowed_providers %}disabled{% endif %}"
data-tab="email"
{% if allowed_providers %}
data-content="{{ text_email_disabled }}"
aria-label="{{ text_email_disabled }}"
data-content="{{ text_email_disabled }}"
aria-label="{{ text_email_disabled }}"
{% endif %}>
<i class="fad fa-envelope icon"></i>
{% block authentication_email_text %}{% trans "Log in with email" %}{% endblock %}
</a>

{% if project_password_url %}
<a class="item" href="{{ project_password_url }}">
<i class="fad fa-lock icon"></i>
{% trans "Use a password" %}
</a>
<a class="item" href="{{ project_password_url }}">
<i class="fad fa-lock icon"></i>
{% trans "Use a password" %}
</a>
{% endif %}
</div>

Expand Down
4 changes: 2 additions & 2 deletions readthedocsext/theme/templates/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@

{% block content-wrapper %}
<div class="ui very padded container">
{% block messages %}
{% block notifications %}
{% include "includes/utils/messages.html" %}
{% endblock messages %}
{% endblock notifications %}

{# TODO move admin-bar usage to content-header #}
{% block admin-bar %}{% endblock %}
Expand Down
51 changes: 33 additions & 18 deletions readthedocsext/theme/templates/builds/build_detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
{% endblock %}

{% block content %}
<div class="ui basic segment padded" data-bind="using: BuildDetailView({id: {{ build.pk }}})">
<div class="ui basic segment padded" data-bind="using: BuildDetailView({}, '{% url "build-detail" build.id %}', '{% url "projects-builds-notifications-list" build.project.slug build.id %}')">

{% block build_detail_metadata %}
<div class="ui top attached segment">
Expand Down Expand Up @@ -202,25 +202,40 @@ <h3>{% trans "Build Errors" %}</h3>

{% else %}

{% comment %}
Error list
{% endcomment %}
{% if build.notifications.exists %}
{% 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>
<span>{% trans "For more information on this error, see our documentation." %}</span>
{% block build_detail_notifications %}
{% comment %}
Build notification and error list

This uses ``readthedocs-notification`` directly, so that the build
detail view can handle all of the API requests in one place.

We pass the entire API notification response object into the web
component, which handles the data the same way as the notification
list element.
{% endcomment %}
<div class="ko hidden ui attached fitted inverted basic segment" data-bind="css: { hidden: !has_notifications() }, foreach: notifications">
<readthedocs-notification
inverted=true
csrf-token="{{ csrf_token }}"
data-bind="webcomponent: {notification: $data}">
</readthedocs-notification>
</div>

{% comment %}
Support for old style build errors, without notifications. In these
builds, ``error`` is a string and there is no header, so we use a
generic header instead.
{% endcomment %}
<div class="ko hidden ui attached fitted inverted basic segment" data-bind="css: { hidden: !error() }">
<div class="ui inverted error notification message">
<div class="header">
<i class="fad fa-circle-exclamation icon"></i>
{% trans "There was an error with this build" %}
</div>
{% endif %}
{% endfor %}
{% 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>
<p data-bind="text: error"></p>
</div>
</div>
{% endif %}
{% endblock build_detail_notifications %}

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

Expand Down
17 changes: 0 additions & 17 deletions readthedocsext/theme/templates/includes/utils/messages.html
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,6 @@
page content pane.
{% endcomment %}

{% comment %}
``user_notifications`` comes from a Context Processor.
We need to use a CustomUser to have access to "user.notifications"
See https://docs.djangoproject.com/en/4.2/ref/settings/#std-setting-AUTH_USER_MODEL
{% endcomment %}
{% if user_notifications %}
{% for notification in user_notifications %}
<div class="ui message">
{% comment %}
Add this Xmark here once we implement dismissing notifications.
<i class="fa-duotone fa-circle-xmark close icon"></i>
{% endcomment %}
{{ notification.get_message.get_rendered_body|safe }}
</div>
{% endfor %}
{% endif %}

{% if messages %}
{% for message in messages %}
{% comment %}
Expand Down
Loading

0 comments on commit 0f36e2e

Please sign in to comment.