From 258732f52a2f157704e089ba365b42e9555f871d Mon Sep 17 00:00:00 2001
From: Moritz Strohm <strohm@data-quest.de>
Date: Wed, 19 Oct 2022 13:09:10 +0000
Subject: [PATCH] fix for BIESt 109, closes #109

Closes #109

Merge request studip/studip!832
---
 .../bootstrap/studip_helper_attributes.js     | 18 +++-
 resources/assets/javascripts/lib/gettext.js   |  3 +-
 .../javascripts/lib/personal_notifications.js | 13 ++-
 .../scss/personal-notifications.scss          | 90 ++++++++++---------
 templates/header.php                          | 14 ++-
 .../personal_notifications/notification.php   | 28 +++---
 6 files changed, 96 insertions(+), 70 deletions(-)

diff --git a/resources/assets/javascripts/bootstrap/studip_helper_attributes.js b/resources/assets/javascripts/bootstrap/studip_helper_attributes.js
index 7af9f9a8e18..0cdbaf37b44 100644
--- a/resources/assets/javascripts/bootstrap/studip_helper_attributes.js
+++ b/resources/assets/javascripts/bootstrap/studip_helper_attributes.js
@@ -262,9 +262,19 @@ $(document).on('keydown', '.enter-accessible', function(event) {
     }
 });
 
-$(document).on('click', '[data-toggles]', function (event) {
-    const target = event.currentTarget.dataset.toggles;
-    $(target).toggle();
+$(document).on('click keydown', '[data-toggles]', function (event) {
+    if ((event.type === 'keydown' && event.key === 'Enter') || event.type === 'click') {
+        const target = event.currentTarget.dataset.toggles;
+        //Check if the target is a checkbox. These have to be toggled differently than
+        //other elements:
+        if ($(target).is(':checkbox')) {
+            //Toggle the checked state of the checkbox:
+            $(target).prop('checked', !$(target).prop('checked'));
+        } else {
+            //Do the normal toggle operation:
+            $(target).toggle();
+        }
 
-    event.preventDefault();
+        event.preventDefault();
+    }
 });
diff --git a/resources/assets/javascripts/lib/gettext.js b/resources/assets/javascripts/lib/gettext.js
index 88e726e2da3..0e3eb9aab4f 100644
--- a/resources/assets/javascripts/lib/gettext.js
+++ b/resources/assets/javascripts/lib/gettext.js
@@ -8,9 +8,10 @@ const DEFAULT_LANG_NAME = 'Deutsch';
 const state = getInitialState();
 
 const $gettext = translate.gettext.bind(translate);
+const $ngettext = translate.ngettext.bind(translate);
 const $gettextInterpolate = translate.gettextInterpolate.bind(translate);
 
-export { $gettext, $gettextInterpolate, translate, getLocale, setLocale, getVueConfig };
+export { $gettext, $ngettext, $gettextInterpolate, translate, getLocale, setLocale, getVueConfig };
 
 function getLocale() {
     return state.locale;
diff --git a/resources/assets/javascripts/lib/personal_notifications.js b/resources/assets/javascripts/lib/personal_notifications.js
index f61150a5824..882d9820483 100644
--- a/resources/assets/javascripts/lib/personal_notifications.js
+++ b/resources/assets/javascripts/lib/personal_notifications.js
@@ -1,6 +1,7 @@
 import Favico from 'favico.js';
 import Cache from './cache.js';
 import PageLayout from './page_layout.js';
+import { $gettextInterpolate, $ngettext } from './gettext';
 
 var stack = {};
 var audio_notification = false;
@@ -91,7 +92,7 @@ function process_notifications({ notifications }) {
 
 const PersonalNotifications = {
     initialize () {
-        if ($('#notification_marker').length > 0) {
+        if ($('#notification_marker .count').length > 0) {
             $('#notification_list .notification').map(function() {
                 var data = $(this).data();
                 stack[data.id] = data;
@@ -154,7 +155,7 @@ const PersonalNotifications = {
     },
     update () {
         var count = _.values(stack).length;
-        var old_count = parseInt($('#notification_marker').text(), 10);
+        var old_count = parseInt($('#notification_marker .count').text(), 10);
         var really_new = 0;
         $('#notification_list > ul > li').each(function() {
             if (parseInt($(this).data('timestamp'), 10) > parseInt($('#notification_marker').data('lastvisit'), 10)) {
@@ -172,16 +173,20 @@ const PersonalNotifications = {
         }
         if (count) {
             $('#notification_container').addClass('hoverable');
+            $('#notification_marker').prop('disabled', false);
             if (count > old_count && audio_notification !== false) {
                 audio_notification.play();
             }
         } else {
             $('#notification_container').removeClass('hoverable');
+            $('#notification_marker').prop('disabled', true);
         }
         if (old_count !== count) {
-            $('#notification_marker').text(count);
+            $('#notification_marker .count').text(count);
+            let notification_text = $ngettext('%{ count } Benachrichtigung', '%{ count } Benachrichtigungen', count);
+            $('#notification_marker').attr('title', $gettextInterpolate(notification_text, {count: count}));
             updateFavicon(count);
-            $('#notification_container .mark-all-as-read').toggleClass('notification_hidden', count < 2);
+            $('#notification_container .mark-all-as-read').toggleClass('invisible', count < 2);
         }
     },
     isVisited () {
diff --git a/resources/assets/stylesheets/scss/personal-notifications.scss b/resources/assets/stylesheets/scss/personal-notifications.scss
index d6521235e73..80846866fca 100644
--- a/resources/assets/stylesheets/scss/personal-notifications.scss
+++ b/resources/assets/stylesheets/scss/personal-notifications.scss
@@ -1,15 +1,14 @@
 #notification_marker {
-    width: 48px;
-    margin-left: 0;
-    padding-left: 0;
-    margin-right: 0;
-    padding-right: 0;
-    height: 28px;
+    margin-left: 0px;
+    padding-left: 0px;
+    margin-right: 0px;
+    padding-right: 0px;
+    width: 100%;
+    height: 100%;
     font-size: 0.8em;
     color: $base-color;
     text-align: center;
     line-height: 28px;
-    vertical-align: text-bottom;
     background-color: $dark-gray-color-10;
     border: 1px solid $dark-gray-color-40;
 
@@ -66,6 +65,15 @@
     &.hoverable:hover {
         .list { display: block; }
     }
+
+    #notification_checkbox {
+        display: none;
+    }
+
+    &.hoverable #notification_checkbox:checked + #notification_list {
+        display: block;
+    }
+
     #notification_list {
         z-index: 1001;
         margin-top: 10px;
@@ -79,6 +87,11 @@
         }
     }
     .list {
+
+        ul {
+            list-style-type: none;
+        }
+
         // Creates an arrow pointing from the list to the triggering element
         @include arrow-top-border(10px, $white, 1px, $light-gray-color-80);
 
@@ -122,7 +135,6 @@
             $padding: 5px;
             border-top: thin solid $light-gray-color-60;
             line-height: 20px;
-            display: block;
             height: auto;
             padding: $padding;
             white-space: normal;
@@ -144,26 +156,33 @@
             &:first-child {
                 border-top: 0;
             }
-            .content {
+
+            .main {
                 display: flex;
                 flex-direction: row;
-                flex-wrap: nowrap;
-
-                .avatar {
-                    $avatar-size: 40px;
-                    margin-right: 10px;
-                    margin-left: 0;
-                    background-position: center center;
-                    background-size: 100%;
-                    background-repeat: no-repeat;
-                    width: $avatar-size;
-                    height: $avatar-size;
-                    min-width: $avatar-size;
+
+                .content {
+                    display: flex;
+                    flex-direction: row;
+                    flex-wrap: nowrap;
+                    flex-grow: 1;
+
+                    .avatar {
+                        $avatar-size: 40px;
+                        margin-right: 10px;
+                        margin-left: 0;
+                        background-position: center center;
+                        background-size: 100%;
+                        background-repeat: no-repeat;
+                        width: $avatar-size;
+                        height: $avatar-size;
+                        min-width: $avatar-size;
+                    }
                 }
             }
         }
 
-        a {
+        a:not(.mark-all-as-read) {
             color: $brand-color-dark;
             display: block;
             padding: 0;
@@ -171,9 +190,11 @@
         }
 
         .options {
+            border: 0;
+            background: none;
             cursor: pointer;
-            float: right;
             padding-top: 4px;
+            height: 24px;
             > img {
                 vertical-align: top;
             }
@@ -183,10 +204,11 @@
         .item:hover .options.hidden { visibility: visible; }
     }
 
-    a.mark-all-as-read,
+    a.mark-all-as-read:not(.invisible),
     a.enable-desktop-notifications {
         background-color: $dark-gray-color-15;
         border-bottom: thin solid $dark-gray-color-45;
+        display: block;
         max-height: 31px;
         padding: 5px 5px 5px 14px;
         z-index: 3;
@@ -213,26 +235,6 @@
 
         // Create blind effect to hide/display this links smoothly
         transition: all 300ms;
-
-        &.notification_hidden {
-            border-bottom-width: 0;
-            max-height: 0;
-            opacity: 0;
-            padding-bottom: 0;
-            padding-top: 0;
-            pointer-events: none;
-
-            + .enable-desktop-notifications {
-                // Creates an arrow pointing from the list to the triggering element
-                @include arrow-top-border(10px, $light-gray-color-20, 1px, $light-gray-color-80);
-                &::before,
-                &::after {
-                    left: ($list-width - 30px);
-                    z-index: 2;
-                }
-                margin: 0;
-            }
-        }
     }
     a.enable-desktop-notifications {
         @include background-icon('notification', 'clickable');
diff --git a/templates/header.php b/templates/header.php
index 63602b2fd0e..f07f765c925 100644
--- a/templates/header.php
+++ b/templates/header.php
@@ -85,11 +85,17 @@ if (isset($_COOKIE['navigation-length'])) {
                             $alert = true;
                         }
                     } ?>
-                    <div id="notification_marker"<?= !empty($alert) ? ' class="alert"' : "" ?> title="<?= _("Benachrichtigungen") ?>" data-lastvisit="<?= $lastvisit ?>">
-                        <?= count($notifications) ?>
-                    </div>
+                    <button id="notification_marker" data-toggles="#notification_checkbox" <?= !empty($alert) ? ' class="alert"' : "" ?>
+                            title="<?= sprintf(
+                                ngettext('%u Benachrichtigung', '%u Benachrichtigungen', count($notifications)),
+                                count($notifications)
+                            ) ?>" data-lastvisit="<?= $lastvisit ?>"
+                            <?= count($notifications) == 0 ? 'disabled' : '' ?>>
+                        <span class="count" aria-hidden="true"><?= count($notifications) ?></span>
+                    </button>
+                    <input type="checkbox" id="notification_checkbox">
                     <div class="list below" id="notification_list">
-                        <a class="mark-all-as-read <? if (count($notifications) < 2) echo 'notification_hidden'; ?>" href="<?= URLHelper::getLink('dispatch.php/jsupdater/mark_notification_read/all', ['return_to' => $_SERVER['REQUEST_URI']]) ?>">
+                        <a class="mark-all-as-read <? if (count($notifications) < 2) echo 'invisible'; ?>" href="<?= URLHelper::getLink('dispatch.php/jsupdater/mark_notification_read/all', ['return_to' => $_SERVER['REQUEST_URI']]) ?>">
                             <?= _('Alle Benachrichtigungen als gelesen markieren') ?>
                         </a>
                         <a class="enable-desktop-notifications" href="#" style="display: none;">
diff --git a/templates/personal_notifications/notification.php b/templates/personal_notifications/notification.php
index 0dfc63c79dc..8a10ae91a9e 100644
--- a/templates/personal_notifications/notification.php
+++ b/templates/personal_notifications/notification.php
@@ -1,16 +1,18 @@
 <li class="notification item" data-id="<?= $notification['personal_notification_id'] ?>" data-timestamp="<?= (int) $notification['mkdate'] ?>">
-    <a class="options mark_as_read" href="#">
-        <?= Icon::create('decline')->asImg(12, ['title' => _('Als gelesen markieren')]) ?>
-    </a>
-    <a class="content" href="<?= URLHelper::getLink('dispatch.php/jsupdater/mark_notification_read/' . $notification['personal_notification_id']) ?>"<?= $notification['dialog'] ? ' data-dialog' : '' ?>>
-    <? if ($notification['avatar']): ?>
-        <div class="avatar" style="background-image: url(<?= $notification['avatar'] ?>);"></div>
-    <? endif; ?>
-        <?= htmlReady($notification['text']) ?>
-    </a>
-<? if ($notification->more_unseen > 0): ?>
-    <div class="more">
-        <?= htmlReady(sprintf(_('... und %u weitere'), $notification->more_unseen)) ?>
+    <div class="main">
+        <a class="content" href="<?= URLHelper::getLink('dispatch.php/jsupdater/mark_notification_read/' . $notification['personal_notification_id']) ?>"<?= $notification['dialog'] ? ' data-dialog' : '' ?>>
+            <? if ($notification['avatar']): ?>
+                <div class="avatar" style="background-image: url(<?= $notification['avatar'] ?>);"></div>
+            <? endif ?>
+            <?= htmlReady($notification['text']) ?>
+        </a>
+        <button class="options mark_as_read">
+            <?= Icon::create('decline')->asImg(12, ['title' => _('Als gelesen markieren')]) ?>
+        </button>
     </div>
-<? endif; ?>
+    <? if ($notification->more_unseen > 0): ?>
+        <div class="more">
+            <?= htmlReady(sprintf(_('... und %u weitere'), $notification->more_unseen)) ?>
+        </div>
+    <? endif ?>
 </li>
-- 
GitLab