Fix dismissable notifications (pypi#3372)

* Fallback to '-1' if there is no version * Correctly add the --visible class regardless of whether it's ephemeral or not * Make the flash messages ephemeral * Make it a little easier to enable pypi-theme
aalmazan · Mar 22, 2018 · 0356350 · 0356350
1 parent f3e914e
commit 0356350
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 11 deletions.
diff --git a/dev/environment b/dev/environment
@@ -1,10 +1,9 @@
 WAREHOUSE_ENV=development
 WAREHOUSE_TOKEN=insecuretoken
 
-# Uncomment the below line if you're working on the PyPI theme, this is a
-# private repository due to the fact that other people's IP is contained in
-# it.
-# WAREHOUSE_THEME: pypi_theme.pypi
+# Uncomment the line below if you're working on pypi-theme, this is a private
+# repository due to the fact that other people's IP is contained in it.
+#WAREHOUSE_THEME=pypi_theme.pypi
 
 AMQP_URL=amqp://guest@rabbitmq:5672//
 

diff --git a/docker-compose.yml b/docker-compose.yml
@@ -29,6 +29,10 @@ services:
       args:
         DEVEL: "yes"
         IPYTHON: "no"
+        # Uncomment the line below and add the private repo URL if you're
+        # working on pypi-theme, this is a private repository due to the fact
+        # that other people's IP is contained in it.
+        #THEME_REPO:
     command: hupper -m twisted --log-format text web -p tcp:port=8000 --wsgi warehouse.wsgi.application
     env_file: dev/environment
     volumes:

diff --git a/warehouse/static/js/warehouse/controllers/notification_controller.js b/warehouse/static/js/warehouse/controllers/notification_controller.js
@@ -27,8 +27,8 @@ export default class extends Controller {
    */
   _getNotificationId() {
     /** Get data from `data-notification-version` attribute */
-    const version = this.data.get("version");
-    if (this.notificationTarget.id && version) {
+    const version = this.data.get("version") || "-1";
+    if (this.notificationTarget.id) {
       return `${this.notificationTarget.id}_${version}__dismissed`;
     }
   }
@@ -37,7 +37,10 @@ export default class extends Controller {
     const notificationId = this._getNotificationId();
     const isDismissable = this.notificationTarget.classList.contains("notification-bar--dismissable");
 
-    if (isDismissable && notificationId && !localStorage.getItem(notificationId)) {
+    // Check if the target is dismissable, and if so:
+    // * whether it has no notificationId (it's ephemeral)
+    // * or it's not in localStorage (it hasn't been dismissed yet)
+    if (isDismissable && (!notificationId || !localStorage.getItem(notificationId))) {
       this.notificationTarget.classList.add("notification-bar--visible");
     }
   }
@@ -49,4 +52,4 @@ export default class extends Controller {
     }
     this.notificationTarget.classList.remove("notification-bar--visible");
   }
-}
+}
diff --git a/warehouse/templates/includes/flash-messages.html b/warehouse/templates/includes/flash-messages.html
@@ -13,21 +13,21 @@
 -#}
 
 {% for message in request.session.pop_flash(queue="error") %}
-<div class="notification-bar notification-bar--danger" data-controller="notification" data-target="notification.notification">
+<div class="notification-bar notification-bar--danger notification-bar--dismissable" data-controller="notification" data-target="notification.notification">
   <span class="notification-bar__message">{{ message }}</span>
   <button title="Dismiss this notification" data-target="notification.dismissButton" data-action="click->notification#dismiss" class="notification-bar__dismiss" aria-label="close"><i class="fa fa-times" aria-hidden="true"></i></button>
 </div>
 {% endfor %}
 
 {% for message in request.session.pop_flash() %}
-<div class="notification-bar notification-bar" data-controller="notification" data-target="notification.notification">
+<div class="notification-bar notification-bar--dismissable" data-controller="notification" data-target="notification.notification">
   <span class="notification-bar__message">{{ message }}</span>
   <button title="Dismiss this notification" data-target="notification.dismissButton" data-action="click->notification#dismiss" class="notification-bar__dismiss" aria-label="close"><i class="fa fa-times" aria-hidden="true"></i></button>
 </div>
 {% endfor %}
 
 {% for message in request.session.pop_flash(queue="success") %}
-<div class="notification-bar notification-bar--success" data-controller="notification" data-target="notification.notification">
+<div class="notification-bar notification-bar--success notification-bar--dismissable" data-controller="notification" data-target="notification.notification">
   <span class="notification-bar__message">{{ message }}</span>
   <button title="Dismiss this notification" data-target="notification.dismissButton" data-action="click->notification#dismiss" class="notification-bar__dismiss" aria-label="dismiss"><i class="fa fa-times" aria-hidden="true"></i></button>
 </div>