From 6389a61fa209f305882beb21786a60070ee8ef23 Mon Sep 17 00:00:00 2001
From: Jiro Ghianni <jiro@maykinmedia.nl>
Date: Tue, 25 Jun 2024 17:40:55 +0200
Subject: [PATCH 1/3] [#2379] Made Search feedback notification focusable for
 screenreaders

---
 src/open_inwoner/cms/cases/tests/test_htmx.py             | 2 +-
 .../components/templates/components/Form/Error.html       | 2 +-
 .../templates/components/Notification/Notification.html   | 4 ++--
 src/open_inwoner/js/components/notifications/index.js     | 8 ++++++--
 src/open_inwoner/scss/components/Grid/Grid.scss           | 6 ++++++
 .../scss/components/Notification/_Notification.scss       | 6 ++++++
 src/open_inwoner/templates/pages/search.html              | 2 +-
 7 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/src/open_inwoner/cms/cases/tests/test_htmx.py b/src/open_inwoner/cms/cases/tests/test_htmx.py
index 5f4c3531a6..b968d61fe1 100644
--- a/src/open_inwoner/cms/cases/tests/test_htmx.py
+++ b/src/open_inwoner/cms/cases/tests/test_htmx.py
@@ -594,7 +594,7 @@ def mock_upload(request, context):
         upload_form = page.locator("#document-upload")
         file_input = upload_form.get_by_label("Sleep of selecteer bestanden")
         submit_button = upload_form.get_by_role("button", name=_("Upload documenten"))
-        notification_list = page.get_by_role("alert").get_by_role("list")
+        notification_list = page.locator(".notification").get_by_role("list")
         notification_list_items = notification_list.get_by_role("listitem")
         file_list = page.get_by_role("list").last
         file_list_items = file_list.get_by_role("listitem")
diff --git a/src/open_inwoner/components/templates/components/Form/Error.html b/src/open_inwoner/components/templates/components/Form/Error.html
index 8d90046f0d..6542a11f9a 100644
--- a/src/open_inwoner/components/templates/components/Form/Error.html
+++ b/src/open_inwoner/components/templates/components/Form/Error.html
@@ -4,7 +4,7 @@
     <div class="notifications notifications__errors">
         <div class="notification notification--warning">
             {% icon icon="warning_amber" icon_position="before" outlined=True %}
-            <div class="notification__content">
+            <div class="notification__content" tabindex="-1">
                 <p class="utrecht-paragraph utrecht-paragraph--oip utrecht-paragraph--oip-compact">{{ message.message }}</p>
             </div>
         </div>
diff --git a/src/open_inwoner/components/templates/components/Notification/Notification.html b/src/open_inwoner/components/templates/components/Notification/Notification.html
index cea1fa2d00..ecb6d30c75 100644
--- a/src/open_inwoner/components/templates/components/Notification/Notification.html
+++ b/src/open_inwoner/components/templates/components/Notification/Notification.html
@@ -1,12 +1,12 @@
 {% load i18n utils button_tags icon_tags button_tags icon_tags %}
-<div class="notification{% if contents %} notification--contents{% endif %}{% if type %} notification--{{ type }}{% endif %}{% if compact %} notification--compact{% endif %} {% if ctx %}notification--{{ ctx }}{% endif %}" role="alert">
+<div class="notification{% if contents %} notification--contents{% endif %}{% if type %} notification--{{ type }}{% endif %}{% if compact %} notification--compact{% endif %} {% if ctx %}notification--{{ ctx }}{% endif %}">
     {% if not icon == False %}
         <div class="notification__icon">
             {% icon icon outlined=True %}
         </div>
     {% endif %}
 
-    <div class="notification__content">
+    <div class="notification__content" tabindex="-1">
         {% if title %}<h2 class="utrecht-heading-2">{{ title }}</h2>{% endif %}
         {% if notification %}<p class="utrecht-paragraph">{{ notification }}</p>{% endif %}
         {% if action %}{% button href=action text=action_text %}{% endif %}
diff --git a/src/open_inwoner/js/components/notifications/index.js b/src/open_inwoner/js/components/notifications/index.js
index b56267d960..1c213cb68c 100644
--- a/src/open_inwoner/js/components/notifications/index.js
+++ b/src/open_inwoner/js/components/notifications/index.js
@@ -25,10 +25,12 @@ export class Notification {
   }
 
   /**
-   * Scrolls to the notification content.
+   * Scrolls to the notification content and sets focus.
    */
   scrollToNotification() {
-    const notificationContent = document.querySelector('.notification__content')
+    const notificationContent = this.node.querySelector(
+      '.notification__content'
+    )
 
     if (notificationContent) {
       // If errors are present, scroll and trigger the opened state
@@ -36,6 +38,8 @@ export class Notification {
         block: 'center',
         behavior: 'smooth',
       })
+      // Set focus for screen readers
+      notificationContent.focus()
     }
   }
 
diff --git a/src/open_inwoner/scss/components/Grid/Grid.scss b/src/open_inwoner/scss/components/Grid/Grid.scss
index 75032c9d49..c84a2fda01 100644
--- a/src/open_inwoner/scss/components/Grid/Grid.scss
+++ b/src/open_inwoner/scss/components/Grid/Grid.scss
@@ -35,6 +35,12 @@
         }
       }
     }
+
+    /// Search grid
+    &--search {
+      display: flex;
+      flex-direction: column;
+    }
   }
 
   &--limit &__sidebar,
diff --git a/src/open_inwoner/scss/components/Notification/_Notification.scss b/src/open_inwoner/scss/components/Notification/_Notification.scss
index 89f5e3212a..27d879bc8c 100644
--- a/src/open_inwoner/scss/components/Notification/_Notification.scss
+++ b/src/open_inwoner/scss/components/Notification/_Notification.scss
@@ -101,6 +101,12 @@
   & &__content {
     margin-top: var(--spacing-tiny);
 
+    &:focus,
+    &:focus-visible {
+      outline: none;
+      border: none;
+    }
+
     & * {
       margin: 0;
     }
diff --git a/src/open_inwoner/templates/pages/search.html b/src/open_inwoner/templates/pages/search.html
index 08db380435..70ef7b8f5c 100644
--- a/src/open_inwoner/templates/pages/search.html
+++ b/src/open_inwoner/templates/pages/search.html
@@ -28,7 +28,7 @@ <h2 class="utrecht-heading-2">{% trans "Zoekfilters" %}</h2>
         {#  end search filters #}
         </aside>
 
-        <div class="grid__main">
+        <div class="grid__main grid__main--search">
             {% if paginator.count %}
                 <div class="search-results">
                     <h2 class="utrecht-heading-2 search-results__title">

From 9ddd2cbd15cb17e7a24ea88a8107d73f82eb320d Mon Sep 17 00:00:00 2001
From: Jiro Ghianni <jiro@maykinmedia.nl>
Date: Thu, 4 Jul 2024 15:44:30 +0200
Subject: [PATCH 2/3] [#2379] Reordered notifications order + forced focus on
 1st element

---
 .../templates/components/Form/Error.html      |  2 +-
 .../components/Notification/Notification.html |  9 +-
 .../Notification/Notifications.html           |  4 +-
 .../conf/locale/nl/LC_MESSAGES/django.po      |  4 +-
 src/open_inwoner/js/components/index.js       |  2 +
 .../notifications/NotificationsList.js        | 39 +++++++++
 .../js/components/notifications/index.js      | 87 ++++++++++++++++---
 .../Notification/_Notifications.scss          |  2 +-
 .../search/tests/test_feedback.py             |  2 +-
 src/open_inwoner/search/views.py              |  2 +-
 10 files changed, 132 insertions(+), 21 deletions(-)
 create mode 100644 src/open_inwoner/js/components/notifications/NotificationsList.js

diff --git a/src/open_inwoner/components/templates/components/Form/Error.html b/src/open_inwoner/components/templates/components/Form/Error.html
index 6542a11f9a..d426cee805 100644
--- a/src/open_inwoner/components/templates/components/Form/Error.html
+++ b/src/open_inwoner/components/templates/components/Form/Error.html
@@ -4,7 +4,7 @@
     <div class="notifications notifications__errors">
         <div class="notification notification--warning">
             {% icon icon="warning_amber" icon_position="before" outlined=True %}
-            <div class="notification__content" tabindex="-1">
+            <div class="notification__content" role="alert" tabindex="-1">
                 <p class="utrecht-paragraph utrecht-paragraph--oip utrecht-paragraph--oip-compact">{{ message.message }}</p>
             </div>
         </div>
diff --git a/src/open_inwoner/components/templates/components/Notification/Notification.html b/src/open_inwoner/components/templates/components/Notification/Notification.html
index ecb6d30c75..0955a3508f 100644
--- a/src/open_inwoner/components/templates/components/Notification/Notification.html
+++ b/src/open_inwoner/components/templates/components/Notification/Notification.html
@@ -6,7 +6,14 @@
         </div>
     {% endif %}
 
-    <div class="notification__content" tabindex="-1">
+    <div class="notification__content"
+         {% if type == "error" %}
+            role="alert"
+         {% elif type == "warning" %}
+            role="alert"
+         {% else %}
+            role="status"
+         {% endif %} tabindex="-1">
         {% if title %}<h2 class="utrecht-heading-2">{{ title }}</h2>{% endif %}
         {% if notification %}<p class="utrecht-paragraph">{{ notification }}</p>{% endif %}
         {% if action %}{% button href=action text=action_text %}{% endif %}
diff --git a/src/open_inwoner/components/templates/components/Notification/Notifications.html b/src/open_inwoner/components/templates/components/Notification/Notifications.html
index 7e497df784..5965d73b82 100644
--- a/src/open_inwoner/components/templates/components/Notification/Notifications.html
+++ b/src/open_inwoner/components/templates/components/Notification/Notifications.html
@@ -1,6 +1,6 @@
 {% load notification_tags string_tags %}
 
-<div class="notifications">
+<section class="notifications">
     {% for message in messages %}
         {% with as_markdown=message.extra_tags|is_substring:"as_markdown" %}
             {% with local_message=message.extra_tags|is_substring:"local_message" %}
@@ -14,4 +14,4 @@
             {% endwith %}
         {% endwith %}
     {% endfor %}
-</div>
+</section>
diff --git a/src/open_inwoner/conf/locale/nl/LC_MESSAGES/django.po b/src/open_inwoner/conf/locale/nl/LC_MESSAGES/django.po
index 11deff964f..2edbf4051d 100644
--- a/src/open_inwoner/conf/locale/nl/LC_MESSAGES/django.po
+++ b/src/open_inwoner/conf/locale/nl/LC_MESSAGES/django.po
@@ -6552,9 +6552,9 @@ msgstr "Zoekindex opnieuw opbouwen"
 #: open_inwoner/search/tests/test_feedback.py:192
 #: open_inwoner/search/views.py:158
 msgid ""
-"Thank you for your feedback. It will help us to improve our search engine"
+"Thank you for your feedback, it will help us improve our search engine."
 msgstr ""
-"Dank u voor uw feedback, hiermee kunnen wij de omgeving verder verbeteren"
+"Dank u voor uw feedback, hiermee kunnen wij de omgeving verder verbeteren."
 
 #: open_inwoner/search/tests/test_logging.py:33 open_inwoner/search/views.py:63
 #, python-brace-format
diff --git a/src/open_inwoner/js/components/index.js b/src/open_inwoner/js/components/index.js
index 26a0e38111..6e743e14ff 100644
--- a/src/open_inwoner/js/components/index.js
+++ b/src/open_inwoner/js/components/index.js
@@ -22,6 +22,7 @@ import './header'
 import './map'
 import './message-file'
 import { Notification } from './notifications'
+import { NotificationsList } from './notifications/NotificationsList'
 import './plans'
 import './plan-preview'
 import './questionnaire'
@@ -55,6 +56,7 @@ const elementWrappers = [
     (elt) => new DisableContactFormButton(elt),
   ],
   [Notification.selector, (elt) => new Notification(elt)],
+  [NotificationsList.selector, (elt) => new NotificationsList(elt)],
   [AnchorMobile.selector, (elt) => new AnchorMobile(elt)],
   [StatusAccordion.selector, (elt) => new StatusAccordion(elt)],
   [FileInput.selector, (elt) => new FileInput(elt)],
diff --git a/src/open_inwoner/js/components/notifications/NotificationsList.js b/src/open_inwoner/js/components/notifications/NotificationsList.js
new file mode 100644
index 0000000000..0c7899639f
--- /dev/null
+++ b/src/open_inwoner/js/components/notifications/NotificationsList.js
@@ -0,0 +1,39 @@
+/**
+ * Surrounding Notifications class.
+ * @class
+ */
+export class NotificationsList {
+  static selector = '.notifications'
+  constructor(notificationContents) {
+    this.notificationContents = notificationContents
+  }
+
+  scrollToFirstNotification() {
+    if (this.notificationContents.length > 0) {
+      // Scroll to the first notification, since there could be multiple
+      this.notificationContents[0].scrollIntoView({
+        block: 'center',
+        behavior: 'smooth',
+      })
+
+      // Add a delay before setting focus for screen readers
+      setTimeout(() => {
+        // Set focus on the first notification content
+        this.notificationContents[0].focus()
+      }, 100)
+    }
+  }
+}
+
+// Start!
+
+const notificationContents = document.querySelectorAll('.notification__content')
+
+// Instantiate notifications section
+document
+  .querySelectorAll(NotificationsList.selector)
+  .forEach((notifications) => new NotificationsList(notifications))
+
+// Focus only the first notification content
+const scrollManager = new NotificationsList(notificationContents)
+scrollManager.scrollToFirstNotification()
diff --git a/src/open_inwoner/js/components/notifications/index.js b/src/open_inwoner/js/components/notifications/index.js
index 1c213cb68c..158db58ee5 100644
--- a/src/open_inwoner/js/components/notifications/index.js
+++ b/src/open_inwoner/js/components/notifications/index.js
@@ -1,5 +1,32 @@
+const typeOrder = ['error', 'warning', 'success', 'info']
+
+/**
+ * Helper function to determine the order index of a notification type.
+ * @param {HTMLElement} notification - The notification element.
+ * @returns {number} - Order index of the notification type.
+ */
+const getTypeOrderIndex = (notification) => {
+  const type = getTypeFromNotification(notification)
+  return typeOrder.indexOf(type)
+}
+
+/**
+ * Helper function to get the type of a notification.
+ * @param {HTMLElement} notification - The notification element.
+ * @returns {string} - Type of the notification.
+ */
+const getTypeFromNotification = (notification) => {
+  let notificationType = ''
+  notification.classList.forEach((cls) => {
+    if (cls.startsWith('notification--')) {
+      notificationType = cls.replace('notification--', '')
+    }
+  })
+  return notificationType
+}
+
 /**
- * Notification class.
+ * Single Notification class.
  * @class
  */
 export class Notification {
@@ -14,6 +41,7 @@ export class Notification {
     this.node = node
 
     this.bindEvents()
+    this.reorderNotifications()
   }
 
   /**
@@ -28,18 +56,23 @@ export class Notification {
    * Scrolls to the notification content and sets focus.
    */
   scrollToNotification() {
-    const notificationContent = this.node.querySelector(
-      '.notification__content'
+    const notificationContents = Array.from(
+      this.node.querySelectorAll('.notification__content')
     )
 
-    if (notificationContent) {
-      // If errors are present, scroll and trigger the opened state
-      notificationContent.scrollIntoView({
-        block: 'center',
-        behavior: 'smooth',
+    if (notificationContents) {
+      notificationContents.forEach((content) => {
+        // If errors are present, scroll and trigger the opened state
+        content.scrollIntoView({
+          block: 'center',
+          behavior: 'smooth',
+        })
+
+        // Add a pause before setting focus for screen readers after DOM load
+        setTimeout(() => {
+          content.focus()
+        }, 100)
       })
-      // Set focus for screen readers
-      notificationContent.focus()
     }
   }
 
@@ -51,8 +84,6 @@ export class Notification {
       e.preventDefault()
       this.close()
     })
-
-    this.scrollToNotification()
   }
 
   /**
@@ -61,6 +92,29 @@ export class Notification {
   close() {
     this.node.parentElement.removeChild(this.node)
   }
+
+  /**
+   * Reorders notifications based on type.
+   */
+  reorderNotifications() {
+    // Get all notifications in the parent container
+    const notificationsContainer = document.querySelector('.notifications')
+    const notifications = Array.from(
+      notificationsContainer.querySelectorAll(Notification.selector)
+    )
+
+    // Sort notifications based on type order
+    notifications.sort((a, b) => {
+      const typeA = getTypeOrderIndex(a)
+      const typeB = getTypeOrderIndex(b)
+      return typeA - typeB
+    })
+
+    // Re-append sorted notifications to parent container
+    notifications.forEach((notification) =>
+      notificationsContainer.appendChild(notification)
+    )
+  }
 }
 
 // Start!
@@ -69,3 +123,12 @@ export class Notification {
 document
   .querySelectorAll(Notification.selector)
   .forEach((notification) => new Notification(notification))
+
+// Scroll to the notifications after reordering
+setTimeout(() => {
+  const firstNotification = document.querySelector(Notification.selector)
+  if (firstNotification) {
+    const instance = new Notification(firstNotification)
+    instance.scrollToNotification()
+  }
+}, 0)
diff --git a/src/open_inwoner/scss/components/Notification/_Notifications.scss b/src/open_inwoner/scss/components/Notification/_Notifications.scss
index 114bc1a600..3e49bc7461 100644
--- a/src/open_inwoner/scss/components/Notification/_Notifications.scss
+++ b/src/open_inwoner/scss/components/Notification/_Notifications.scss
@@ -3,7 +3,7 @@
   top: var(--spacing-extra-large);
   display: flex;
   flex-direction: column;
-  gap: var(--spacing-extra-large);
+  gap: var(--spacing-medium);
   z-index: 1002;
 
   /// Multiple errors.
diff --git a/src/open_inwoner/search/tests/test_feedback.py b/src/open_inwoner/search/tests/test_feedback.py
index 91407d4d0a..506fdeada2 100644
--- a/src/open_inwoner/search/tests/test_feedback.py
+++ b/src/open_inwoner/search/tests/test_feedback.py
@@ -189,6 +189,6 @@ def test_feedback_form_not_displayed_after_submit(self):
         self.assertEqual(
             message.message,
             _(
-                "Thank you for your feedback. It will help us to improve our search engine"
+                "Thank you for your feedback, it will help us improve our search engine."
             ),
         )
diff --git a/src/open_inwoner/search/views.py b/src/open_inwoner/search/views.py
index e3d5141711..37965d77ee 100644
--- a/src/open_inwoner/search/views.py
+++ b/src/open_inwoner/search/views.py
@@ -155,7 +155,7 @@ def form_valid(self, form):
             self.request,
             messages.SUCCESS,
             _(
-                "Thank you for your feedback. It will help us to improve our search engine"
+                "Thank you for your feedback, it will help us improve our search engine."
             ),
         )
         redirect = furl(reverse("search:search"))

From 2f6998e76b1ffee65ffd2708922f2bf86a674e93 Mon Sep 17 00:00:00 2001
From: Jiro Ghianni <jiro@maykinmedia.nl>
Date: Mon, 8 Jul 2024 17:55:37 +0200
Subject: [PATCH 3/3] [#2379] Cleaned up notification code + improved login
 form

---
 .../js/components/form/LoginForm.js           |  7 ++++
 src/open_inwoner/js/components/index.js       |  2 -
 .../notifications/NotificationsList.js        | 39 -------------------
 .../js/components/notifications/index.js      | 36 ++++++++++-------
 4 files changed, 29 insertions(+), 55 deletions(-)
 delete mode 100644 src/open_inwoner/js/components/notifications/NotificationsList.js

diff --git a/src/open_inwoner/js/components/form/LoginForm.js b/src/open_inwoner/js/components/form/LoginForm.js
index af1f94779d..1b778656ac 100644
--- a/src/open_inwoner/js/components/form/LoginForm.js
+++ b/src/open_inwoner/js/components/form/LoginForm.js
@@ -20,9 +20,16 @@ export class LoginFormFocus {
   }
 
   hideLoginFormOnLoad() {
+    const notificationContent = this.loginFormColumn.querySelector(
+      '.notification__content'
+    )
     if (this.loginFormColumn) {
       this.loginFormColumn.classList.add('hide')
     }
+    // Show form on error
+    if (notificationContent) {
+      this.loginFormColumn.classList.remove('hide')
+    }
   }
 
   addEmailToggleListener() {
diff --git a/src/open_inwoner/js/components/index.js b/src/open_inwoner/js/components/index.js
index 6e743e14ff..26a0e38111 100644
--- a/src/open_inwoner/js/components/index.js
+++ b/src/open_inwoner/js/components/index.js
@@ -22,7 +22,6 @@ import './header'
 import './map'
 import './message-file'
 import { Notification } from './notifications'
-import { NotificationsList } from './notifications/NotificationsList'
 import './plans'
 import './plan-preview'
 import './questionnaire'
@@ -56,7 +55,6 @@ const elementWrappers = [
     (elt) => new DisableContactFormButton(elt),
   ],
   [Notification.selector, (elt) => new Notification(elt)],
-  [NotificationsList.selector, (elt) => new NotificationsList(elt)],
   [AnchorMobile.selector, (elt) => new AnchorMobile(elt)],
   [StatusAccordion.selector, (elt) => new StatusAccordion(elt)],
   [FileInput.selector, (elt) => new FileInput(elt)],
diff --git a/src/open_inwoner/js/components/notifications/NotificationsList.js b/src/open_inwoner/js/components/notifications/NotificationsList.js
deleted file mode 100644
index 0c7899639f..0000000000
--- a/src/open_inwoner/js/components/notifications/NotificationsList.js
+++ /dev/null
@@ -1,39 +0,0 @@
-/**
- * Surrounding Notifications class.
- * @class
- */
-export class NotificationsList {
-  static selector = '.notifications'
-  constructor(notificationContents) {
-    this.notificationContents = notificationContents
-  }
-
-  scrollToFirstNotification() {
-    if (this.notificationContents.length > 0) {
-      // Scroll to the first notification, since there could be multiple
-      this.notificationContents[0].scrollIntoView({
-        block: 'center',
-        behavior: 'smooth',
-      })
-
-      // Add a delay before setting focus for screen readers
-      setTimeout(() => {
-        // Set focus on the first notification content
-        this.notificationContents[0].focus()
-      }, 100)
-    }
-  }
-}
-
-// Start!
-
-const notificationContents = document.querySelectorAll('.notification__content')
-
-// Instantiate notifications section
-document
-  .querySelectorAll(NotificationsList.selector)
-  .forEach((notifications) => new NotificationsList(notifications))
-
-// Focus only the first notification content
-const scrollManager = new NotificationsList(notificationContents)
-scrollManager.scrollToFirstNotification()
diff --git a/src/open_inwoner/js/components/notifications/index.js b/src/open_inwoner/js/components/notifications/index.js
index 158db58ee5..5627febab9 100644
--- a/src/open_inwoner/js/components/notifications/index.js
+++ b/src/open_inwoner/js/components/notifications/index.js
@@ -63,6 +63,8 @@ export class Notification {
     if (notificationContents) {
       notificationContents.forEach((content) => {
         // If errors are present, scroll and trigger the opened state
+        // The document.querySelectorAll method returns elements in the order they appear in the document,
+        // so the forEach method will create Notification instances in this same order.
         content.scrollIntoView({
           block: 'center',
           behavior: 'smooth',
@@ -97,23 +99,29 @@ export class Notification {
    * Reorders notifications based on type.
    */
   reorderNotifications() {
-    // Get all notifications in the parent container
+    // Select the parent container, in order to re-index its children
     const notificationsContainer = document.querySelector('.notifications')
-    const notifications = Array.from(
-      notificationsContainer.querySelectorAll(Notification.selector)
-    )
 
-    // Sort notifications based on type order
-    notifications.sort((a, b) => {
-      const typeA = getTypeOrderIndex(a)
-      const typeB = getTypeOrderIndex(b)
-      return typeA - typeB
-    })
+    if (notificationsContainer) {
+      const notifications = Array.from(
+        // Get first matching element in the document
+        notificationsContainer.querySelectorAll(Notification.selector)
+      )
+
+      // Re-indexing the NodeList: Sort notifications (children/siblings) based on type order
+      notifications.sort((a, b) => {
+        const typeA = getTypeOrderIndex(a)
+        const typeB = getTypeOrderIndex(b)
+        return typeA - typeB
+      })
 
-    // Re-append sorted notifications to parent container
-    notifications.forEach((notification) =>
-      notificationsContainer.appendChild(notification)
-    )
+      // Re-append sorted notifications to parent container
+      notifications.forEach((notification) =>
+        notificationsContainer.appendChild(notification)
+      )
+    } else {
+      return
+    }
   }
 }