From 5ad1fb5b6dd0a801e2f5d9fc6c0d59db831a4fd0 Mon Sep 17 00:00:00 2001
From: Ron Lucke <lucke@elan-ev.de>
Date: Mon, 13 Jan 2025 20:00:58 +0100
Subject: [PATCH] fix #5107

---
 .../javascripts/lib/personal_notifications.js |   6 +-
 .../assets/stylesheets/mixins/colors.scss     |   2 +
 .../assets/stylesheets/scss/globalsearch.scss |   6 +-
 resources/assets/stylesheets/scss/header.scss |  41 ++---
 .../scss/personal-notifications.scss          |  28 ++--
 .../assets/stylesheets/scss/responsive.scss   |  22 +--
 .../assets/stylesheets/scss/variables.scss    |   2 +
 templates/header.php                          | 154 +++++++++---------
 tests/e2e/login.spec.ts                       |   2 +-
 tests/e2e/logout.spec.ts                      |   2 +-
 10 files changed, 123 insertions(+), 142 deletions(-)

diff --git a/resources/assets/javascripts/lib/personal_notifications.js b/resources/assets/javascripts/lib/personal_notifications.js
index 798566fae0f..a70e82e6456 100644
--- a/resources/assets/javascripts/lib/personal_notifications.js
+++ b/resources/assets/javascripts/lib/personal_notifications.js
@@ -170,11 +170,11 @@ const PersonalNotifications = {
         if (really_new > 0) {
             $('#notification_marker')
                 .data('seen', false);
-            $('#avatar-menu-container')
+            $('#notification-wrapper')
                 .addClass('alert');
             PageLayout.title_prefix = '(!) ';
         } else {
-            $('#avatar-menu-container').removeClass('alert');
+            $('#notification-wrapper').removeClass('alert');
             PageLayout.title_prefix = '';
         }
         if (count) {
@@ -227,7 +227,7 @@ const PersonalNotifications = {
         $.get(STUDIP.URLHelper.getURL('dispatch.php/jsupdater/notifications_seen')).then(time => {
             $('#notification_marker')
                 .data('lastvisit', time);
-            $('#avatar-menu-container')
+            $('#notification-wrapper')
                 .removeClass('alert');
                 
         });
diff --git a/resources/assets/stylesheets/mixins/colors.scss b/resources/assets/stylesheets/mixins/colors.scss
index cb6bce67e66..249a6dfd09a 100644
--- a/resources/assets/stylesheets/mixins/colors.scss
+++ b/resources/assets/stylesheets/mixins/colors.scss
@@ -323,3 +323,5 @@ $color--info: $color--blue-2;
 $color--info-alternative: mix($color--info, $color--white, 20%);
 
 $color-image-placeholder-background: $color--gray-6;
+
+$color-header-inverted: $color--white;
\ No newline at end of file
diff --git a/resources/assets/stylesheets/scss/globalsearch.scss b/resources/assets/stylesheets/scss/globalsearch.scss
index 0d4052a2ffd..3ccd8ae954a 100644
--- a/resources/assets/stylesheets/scss/globalsearch.scss
+++ b/resources/assets/stylesheets/scss/globalsearch.scss
@@ -4,7 +4,7 @@
 #globalsearch-searchbar {
     display: flex;
     align-items: center;
-    border: thin solid var(--color--font-inverted);
+    border: thin solid var(--color--header-inverted);
     border-radius: var(--border-radius-search);
     background-color: var(--color--brand-primary);
     padding: 3px 5px 2px 5px;
@@ -77,7 +77,7 @@
 
     // List display
     #globalsearch-list {
-        background-color: var(--white);
+        background-color: var(--color--global-background);
         box-shadow: 1px 1px 1px var(--light-gray-color-80);
         color: var(--text-color);
         display: none;
@@ -118,7 +118,7 @@
         background-color: var(--white);
 
         #globalsearch-input {
-            background-color: var(--white);
+            background-color: var(--color--header-inverted);
             color: var(--color--font-primary);
 
             &::placeholder {
diff --git a/resources/assets/stylesheets/scss/header.scss b/resources/assets/stylesheets/scss/header.scss
index 0a04d644561..c16a79047d7 100644
--- a/resources/assets/stylesheets/scss/header.scss
+++ b/resources/assets/stylesheets/scss/header.scss
@@ -45,7 +45,7 @@
             justify-content: space-between;
             list-style-type: none;
             height: 40px;
-            gap: 20px;
+            gap: 8px;
 
             > li {
 
@@ -188,40 +188,26 @@
     padding: 0 5px 0 0;
 }
 
-#avatar-menu-container {
-    display: inline-flex;
-
-    &.header_avatar_container {
-        align-items: end;
-        flex: 0;
-        border-radius: var(--border-radius-avatar-menu);
-        background-color: var(--color--global-background);
-
-        &.alert,
-        &.alert #notification_marker {
-            background-color: var(--color--warning);
-            color: var(--color--font-inverted);
-        }
-    }
-
+#avatar-wrapper {
     #avatar-menu {
-        height: 28px;
+        height: 30px;
+        width: 30px;
         margin: 0;
         z-index: 1003;
 
         .action-menu.avatar-menu {
             z-index: 1002;
-            padding: 1px 1px 1px 0;
 
             .action-menu-icon {
-                height: 26px;
                 position: relative;
-                width: 26px;
+                height: 100%;
+                width: 100%;
                 z-index: 1;
 
                 img {
-                    height: 26px;
-                    width: 26px;
+                    height: 28px;
+                    width: 28px;
+                    border: solid 1px var(--color--header-inverted);
                     border-radius: var(--border-radius-avatar-menu);
                 }
 
@@ -233,7 +219,7 @@
 
             .action-menu-content {
                 position: absolute;
-                top: 41px;
+                top: 40px;
                 right: 0;
                 background: var(--white);
                 box-shadow: 1px 1px 1px var(--dark-gray-color-60);
@@ -255,13 +241,6 @@
             }
         }
     }
-
-    #notification-container + #avatar-menu {
-        .action-menu-icon img {
-            border-top-left-radius: 0;
-            border-bottom-left-radius: 0;
-        }
-    }
 }
 
 // Fix header covering relevant other areas
diff --git a/resources/assets/stylesheets/scss/personal-notifications.scss b/resources/assets/stylesheets/scss/personal-notifications.scss
index ada6054e7ac..2b16e89399e 100644
--- a/resources/assets/stylesheets/scss/personal-notifications.scss
+++ b/resources/assets/stylesheets/scss/personal-notifications.scss
@@ -1,26 +1,34 @@
 #notification_marker {
-    margin-left: 0px;
-    padding-left: 0px;
-    margin-right: 0px;
-    padding-right: 0px;
+    padding: 0 8px;
     width: 100%;
     height: 100%;
-    font-size: 12px;
     color: var(--color--font-primary);
     text-align: center;
-    line-height: 28px;
+    line-height: 30px;
     border: none;
     border-radius: var(--border-radius-avatar-menu);
-    background-color: var(--color--global-background);
+    background-color: var(--color--header-inverted);
+
+    &.alert {
+        background-color: var(--color--warning);
+        color: var(--color--font-inverted);
+    }
+
+    .count {
+        padding: 0 5px;
+    }
+
+    img {
+        vertical-align: middle;
+        margin-top: -3px;
+    }
 }
 
 #notification-container {
     $arrow-height: 10px;
     border-radius: var(--border-radius-avatar-menu);
     $list-width: 400px;
-
-    width: 28px;
-    height: 28px;
+    height: 30px;
     color: var(--base-color);
     vertical-align: text-bottom;
     position: relative;
diff --git a/resources/assets/stylesheets/scss/responsive.scss b/resources/assets/stylesheets/scss/responsive.scss
index 35ffa7c9557..d6c838cc9b3 100644
--- a/resources/assets/stylesheets/scss/responsive.scss
+++ b/resources/assets/stylesheets/scss/responsive.scss
@@ -285,7 +285,8 @@ $sidebarOut: -330px;
     }
 
     #quicksearch_item,
-    #avatar-menu-container,
+    #notification-wrapper,
+    #avatar-wrapper,
     #current-page-structure {
         display: none;
     }
@@ -302,8 +303,8 @@ $sidebarOut: -330px;
             }
         }
 
-        #notification-container,
-        .header_avatar_container,
+        #notification-wrapper,
+        #avatar-wrapper,
         #sidebar-menu {
             display: none;
         }
@@ -760,21 +761,6 @@ html:not(.responsive-display):not(.fullscreen-mode) {
             line-height: 20px;
         }
 
-        #avatar-menu-container {
-            position: relative;
-            bottom: 0px;
-            right: 0px;
-            line-height: 20px !important;
-
-            #avatar-menu {
-                display: none;
-            }
-
-            &::after {
-                display: none !important;
-            }
-        }
-
         #top-bar {
             box-sizing: border-box;
             height: $header-bar-container-height;
diff --git a/resources/assets/stylesheets/scss/variables.scss b/resources/assets/stylesheets/scss/variables.scss
index 2941fe69e3b..c3565dd1acb 100644
--- a/resources/assets/stylesheets/scss/variables.scss
+++ b/resources/assets/stylesheets/scss/variables.scss
@@ -299,6 +299,8 @@ $grid-gap: 0;
 
     --color-image-placeholder-background: #{$color-image-placeholder-background};
 
+    --color--header-inverted: #{$color-header-inverted};
+
     --border-radius-default: #{$border-radius};
     --border-radius-avatar-menu: #{$border-radius-avatar-menu};
     --border-radius-search: #{$border-radius-search};
diff --git a/templates/header.php b/templates/header.php
index 2f1d50992f4..a3ecc999917 100644
--- a/templates/header.php
+++ b/templates/header.php
@@ -122,90 +122,94 @@ if ($navigation) {
             <? $active = Navigation::getItem('/profile')?->isActive() ?? false; ?>
 
             <? if ($GLOBALS['perm']->have_perm('autor')) : ?>
-                <li id="avatar-menu-container"
-                    class="header_avatar_container <?= PersonalNotifications::hasUnseenNotifications() ? 'alert' : '' ?>"
-                >
+
                 <? if (PersonalNotifications::isActivated()): ?>
-                    <? $notifications = PersonalNotifications::getMyNotifications() ?>
-                    <div id="notification-container"  <?= count($notifications) > 0 ? ' class="hoverable"' : '' ?>>
-                        <button id="notification_marker"
-                                data-toggles="#notification_checkbox"
-                                title="<?= sprintf(
-                                    ngettext('%u Benachrichtigung', '%u Benachrichtigungen', count($notifications)),
-                                    count($notifications)
-                                ) ?>"
-                                aria-controls="notification-list"
-                                data-lastvisit="<?= UserConfig::get($GLOBALS['user']->id)->getValue('NOTIFICATIONS_SEEN_LAST_DATE') ?>"
-                                <? if (count($notifications) === 0) echo 'disabled'; ?>
-                                <? if (PersonalNotifications::hasUnseenNotifications()) echo 'class="alert"'; ?>
-                                aria-expanded="false"
-                        >
-                            <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 'invisible'; ?>"
-                               href="<?= URLHelper::getLink('dispatch.php/jsupdater/mark_notification_read/all', ['return_to' => $_SERVER['REQUEST_URI']]) ?>"
+                    <li id="notification-wrapper">
+                        <? $notifications = PersonalNotifications::getMyNotifications() ?>
+                        <div id="notification-container"  <?= count($notifications) > 0 ? ' class="hoverable"' : '' ?>>
+                            <button id="notification_marker"
+                                    data-toggles="#notification_checkbox"
+                                    title="<?= sprintf(
+                                        ngettext('%u Benachrichtigung', '%u Benachrichtigungen', count($notifications)),
+                                        count($notifications)
+                                    ) ?>"
+                                    aria-controls="notification-list"
+                                    data-lastvisit="<?= UserConfig::get($GLOBALS['user']->id)->getValue('NOTIFICATIONS_SEEN_LAST_DATE') ?>"
+                                    <? if (count($notifications) === 0) echo 'disabled'; ?>
+                                    class="<?= PersonalNotifications::hasUnseenNotifications() ? 'alert' : '' ?>"
+                                    aria-expanded="false"
                             >
-                                <?= _('Alle Benachrichtigungen als gelesen markieren') ?>
-                            </a>
-                            <a class="enable-desktop-notifications" href="#" style="display: none;">
-                                <?= _('Desktop-Benachrichtigungen aktivieren') ?>
-                            </a>
-                            <ul>
-                            <? foreach ($notifications as $notification) : ?>
-                                <?= $notification->getLiElement() ?>
-                            <? endforeach ?>
-                            </ul>
+                                <span class="count" aria-hidden="true"><?= count($notifications) ?></span>
+                                <? $icon_role =  PersonalNotifications::hasUnseenNotifications() ? ICON::ROLE_INFO_ALT : ICON::ROLE_CLICKABLE ?>
+                                <?= Icon::create('notification2', $icon_role)->asImg() ?>
+                            </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 '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;">
+                                    <?= _('Desktop-Benachrichtigungen aktivieren') ?>
+                                </a>
+                                <ul>
+                                <? foreach ($notifications as $notification) : ?>
+                                    <?= $notification->getLiElement() ?>
+                                <? endforeach ?>
+                                </ul>
+                            </div>
+                        <? if (PersonalNotifications::isAudioActivated()): ?>
+                            <audio id="audio_notification" preload="none">
+                                <source src="<?= Assets::url('sounds/blubb.ogg') ?>" type="audio/ogg">
+                                <source src="<?= Assets::url('sounds/blubb.mp3') ?>" type="audio/mpeg">
+                            </audio>
+                        <? endif; ?>
                         </div>
-                    <? if (PersonalNotifications::isAudioActivated()): ?>
-                        <audio id="audio_notification" preload="none">
-                            <source src="<?= Assets::url('sounds/blubb.ogg') ?>" type="audio/ogg">
-                            <source src="<?= Assets::url('sounds/blubb.mp3') ?>" type="audio/mpeg">
-                        </audio>
-                    <? endif; ?>
-                    </div>
+                    </li>
                 <? endif; ?>
 
                 <? if (Navigation::hasItem('/avatar')): ?>
-                    <form id="avatar-menu" method="post">
-                    <?php
-                        $action_menu = ContentGroupMenu::get();
-                        $action_menu->addCSSClass('avatar-menu');
-                        $action_menu->addAttribute('data-action-menu-reposition', 'false');
-                        $action_menu->setLabel(User::findCurrent()->getFullName());
-                        $action_menu->setAriaLabel(_('Profilmenü'));
-                        $action_menu->setIcon(
-                            Avatar::getAvatar(User::findCurrent()->id)->getImageTag(),
-                            ['id' => 'header_avatar_image_link']
-                        );
+                    <li id="avatar-wrapper">
+                        <form id="avatar-menu" method="post">
+                        <?php
+                            $action_menu = ContentGroupMenu::get();
+                            $action_menu->addCSSClass('avatar-menu');
+                            $action_menu->addAttribute('data-action-menu-reposition', 'false');
+                            $action_menu->setLabel(User::findCurrent()->getFullName());
+                            $action_menu->setAriaLabel(_('Profilmenü'));
+                            $action_menu->setIcon(
+                                Avatar::getAvatar(User::findCurrent()->id)->getImageTag(),
+                                ['id' => 'header_avatar_image_link']
+                            );
 
-                        foreach (Navigation::getItem('/avatar') as $subnav) {
-                            if ($subnav->getRenderAsButton()) {
-                                $action_menu->addButton(
-                                    'logout',
-                                    $subnav->getTitle(),
-                                    $subnav->getImage(),
-                                    array_merge(
-                                        $subnav->getLinkAttributes(),
-                                        ['formaction' => URLHelper::getURL($subnav->getURL(), [], true)]
-                                    )
-                                );
-                            } else {
-                                $action_menu->addLink(
-                                    URLHelper::getURL($subnav->getURL(), [], true),
-                                    $subnav->getTitle(),
-                                    $subnav->getImage(),
-                                    $subnav->getLinkAttributes()
-                                );
+                            foreach (Navigation::getItem('/avatar') as $subnav) {
+                                if ($subnav->getRenderAsButton()) {
+                                    $action_menu->addButton(
+                                        'logout',
+                                        $subnav->getTitle(),
+                                        $subnav->getImage(),
+                                        array_merge(
+                                            $subnav->getLinkAttributes(),
+                                            ['formaction' => URLHelper::getURL($subnav->getURL(), [], true)]
+                                        )
+                                    );
+                                } else {
+                                    $action_menu->addLink(
+                                        URLHelper::getURL($subnav->getURL(), [], true),
+                                        $subnav->getTitle(),
+                                        $subnav->getImage(),
+                                        $subnav->getLinkAttributes()
+                                    );
+                                }
                             }
-                        }
-                        SkipLinks::addIndex(_('Profilmenü'), 'header_avatar_image_link', 1, false);
-                    ?>
-                    <?= $action_menu->render(); ?>
-                    </form>
+                            SkipLinks::addIndex(_('Profilmenü'), 'header_avatar_image_link', 1, false);
+                        ?>
+                        <?= $action_menu->render(); ?>
+                        </form>
+                    </li>
                 <? endif; ?>
-                </li>
+                
             <? endif; ?>
         <? else: ?>
                 <li>
diff --git a/tests/e2e/login.spec.ts b/tests/e2e/login.spec.ts
index 2e859dd1bba..f646272bb4a 100644
--- a/tests/e2e/login.spec.ts
+++ b/tests/e2e/login.spec.ts
@@ -55,7 +55,7 @@ test.describe('Loggin In - HTML Web Form @auth', () => {
             await benutzername.fill(credentials.autor.username);
             await passwort.fill(credentials.autor.password);
             await submit.click();
-            await expect(page.locator('#avatar-menu-container')).toBeVisible();
+            await expect(page.locator('#notification-wrapper')).toBeVisible();
             await expect(page).toHaveURL(`${baseURL}dispatch.php/start`);
         });
     });
diff --git a/tests/e2e/logout.spec.ts b/tests/e2e/logout.spec.ts
index bd87e0b28da..f969df977ba 100644
--- a/tests/e2e/logout.spec.ts
+++ b/tests/e2e/logout.spec.ts
@@ -7,7 +7,7 @@ test.describe('Logging Out', () => {
 
     test('should take us back to the homepage', async ({ page, baseURL }) => {
         await page.goto(baseURL);
-        await expect(page.locator('#avatar-menu-container')).toBeVisible();
+        await expect(page.locator('#notification-wrapper')).toBeVisible();
         await page.getByTitle('Testaccount Dozent').click();
         await page.getByRole('link', { name: 'Logout' }).click();
         await expect(page).toHaveURL(/index\.php.*logout=true/);
-- 
GitLab