Browse Source

Header: restore the work that missed #5895; menu cache marks the current page's tab (#5903)

* Header: restore the spacing, centring and popup work that missed #5895

#5895 was merged at its third commit; four later commits on its branch never
reached master. This brings back their template and CSS parts:

- 32px between the logo and the main menu, 24px before the toolbar, and the
  menu column may shrink (min-w-0).
- The main menu row is centred between the logo and the toolbar.
- Tabs: icon side padding 4px less so they look even, an icon-only 44px More,
  8px between icon and label; menu icons take the item's text colour.
- More list: 36px rows inset 8px; header popups never run past the window.
- Popups: a shadow edge and soft lift (smooth-shadow-popover) instead of a gray
  ring on top of a shadow.
- Layout rows whose cells are all empty are hidden, so they don't double the
  gap between their neighbours.

The branch's hide-until-measured rule and JS are not restored: #5900 replaced
them (artificer menu.css and BxDolMenuMoreAuto.js).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Menus with content cache mark the current page's item as selected

The cache key of a cached menu ('global', 'guest', 'per_user', 'per_acl') did
not include the selected item, but the cached HTML marks it with
bx-menu-tab-active. The first page to render a menu after the cache was
cleared decided the selected item for every other page: after a visit to the
home page, the header kept Home active on /about and everywhere else.

The key now includes the item the menu shows as selected (module and name,
hashed), or nothing when none of its items is, so pages that select nothing in
the menu still share one entry.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Menu cache key: the selected item's name, not an md5 of it

SonarCloud flags md5 (S4790). The key only needs to tell selected items apart,
and module and item names are identifiers, so the key takes them as they are,
keeping only file-name-safe characters. The helper gets braces and one exit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Header More list: 44px rows like the tabs they continue

The items in 'more' are the same tabs with the same 24px icon and 16px label,
so they now take the tabs' 44px height and 12px/16px side padding instead of
the 36px small size.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Menu cache: keep the item names with the menu; short key suffix

The selected-item lookup queried sys_menu_items before every cache read. The
items' module and name pairs are now kept in the content cache under the
menu's own key (plus '_items'), so a cache hit needs no query and the list goes
stale exactly when the cached menu does. A warm /about makes the same two menu
item queries as master.

The key suffix is the first 16 hex digits of a sha256 of module and name, so a
long item name can't push the cache file name past the file system's limit,
and names that differ only in punctuation stay apart.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Header popups: the height limit also reaches popups moved under body

The toolbar's popups (bx-sliding-menu-*, e.g. Add content) are moved under body
when they open, so the #bx-toolbar selector never capped them and a long Add
content menu ran past the bottom of the window. They are matched by id too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Andrey Yasko 4 days ago
parent
commit
8365ee5289

+ 1 - 1
modules/boonex/artificer/data/template/system/menu_main.html

@@ -2,7 +2,7 @@
     <bx_repeat:menu_items>
         <li class="bx-menu-item bx-menu-item-__name__ __class_add__ rounded-md" __attrs_wrp__>
             <a href="__link__" <bx_if:onclick>onclick="__onclick__"</bx_if:onclick> class="bx-stl-mil group"__attrs__>
-                <div class="bx-menu-item-icon bx-menu-floating-blocks-icon bx-stl-mii group-hover:text-gray-700 dark:group-hover:text-gray-300">
+                <div class="bx-menu-item-icon bx-menu-floating-blocks-icon bx-stl-mii text-current">
                     <bx_include_auto:menu_icon.html />
                 </div>
                 <bx_if:title>

File diff suppressed because it is too large
+ 0 - 0
scripts/tailwind.max.css


+ 12 - 0
tailwind.smooth-shadow.js

@@ -14,6 +14,8 @@
  *  - `smooth-shadow-border` — a shadow in place of a border for surfaces that only need an edge (the
  *    header bar, page blocks): a 1px ring, a 1px lift and a 2px ambient layer in light mode, and a single
  *    white ring under `.dark`, where depth shadows don't show. Never pair it with `border`/`ring`.
+ *  - `smooth-shadow-popover` — the same edge for floating surfaces (popups, dropdown menus), plus a
+ *    soft drop shadow that lifts them off the page in both modes.
  */
 const plugin = require('tailwindcss/plugin');
 const flattenColorPalette = require('tailwindcss/lib/util/flattenColorPalette').default;
@@ -32,6 +34,8 @@ const RING = '0 0 0 var(--smooth-ring-width, 1px) var(--smooth-ring-color)';
 
 const BORDER_LIGHT = '0 0 0 1px oklch(0 0 0 / 0.06), 0 1px 2px -1px oklch(0 0 0 / 0.06), 0 2px 4px 0 oklch(0 0 0 / 0.04)';
 const BORDER_DARK = '0 0 0 1px oklch(1 0 0 / 0.08)';
+const POPOVER_LIGHT = '0 0 0 1px oklch(0 0 0 / 0.06), 0 4px 6px -2px oklch(0 0 0 / 0.05), 0 12px 24px -4px oklch(0 0 0 / 0.10)';
+const POPOVER_DARK = '0 0 0 1px oklch(1 0 0 / 0.10), 0 12px 24px -4px oklch(0 0 0 / 0.50)';
 
 function stack(size, withRing) {
     const layers = LAYERS[size].map(([y, blur, spread, alpha]) =>
@@ -50,14 +54,17 @@ module.exports = plugin(function ({ addBase, addUtilities, matchUtilities, theme
             '--smooth-ring-color': 'rgba(0, 0, 0, 0.05)',
             '--smooth-ring-width': '1px',
             '--smooth-shadow-border': BORDER_LIGHT,
+            '--smooth-shadow-popover': POPOVER_LIGHT,
         },
         '.light, [data-theme="light"]': {
             '--smooth-ring-color': 'rgba(0, 0, 0, 0.05)',
             '--smooth-shadow-border': BORDER_LIGHT,
+            '--smooth-shadow-popover': POPOVER_LIGHT,
         },
         '.dark, [data-theme="dark"]': {
             '--smooth-ring-color': 'rgba(255, 255, 255, 0.18)',
             '--smooth-shadow-border': BORDER_DARK,
+            '--smooth-shadow-popover': POPOVER_DARK,
         },
     });
 
@@ -87,6 +94,11 @@ module.exports = plugin(function ({ addBase, addUtilities, matchUtilities, theme
         '--tw-shadow-colored': 'var(--smooth-shadow-border)',
         boxShadow: BOX_SHADOW,
     };
+    utilities['.smooth-shadow-popover'] = {
+        '--tw-shadow': 'var(--smooth-shadow-popover)',
+        '--tw-shadow-colored': 'var(--smooth-shadow-popover)',
+        boxShadow: BOX_SHADOW,
+    };
 
     addUtilities(utilities);
 

+ 1 - 1
template/_page_toolbar.html

@@ -12,7 +12,7 @@
                         <bx_injection:injection_logo_after />
                     </div>
                     <!-- Menu -->
-                    <div class="bx-toolbar-item bx-ti-menu flex-2 hidden lg:block w-64 ltr:ml-4 rtl:mr-4">
+                    <div class="bx-toolbar-item bx-ti-menu flex-2 hidden lg:block w-64 min-w-0 ltr:ml-8 rtl:mr-8 ltr:mr-6 rtl:ml-6">
                         <nav id="bx-menu-toolbar-1-container" class="bx-toolbar-item bx-ti-menu w-full">
                             <bx_menu:sys_site />
                         </nav>

+ 39 - 2
template/css/tailwind.css

@@ -631,6 +631,10 @@
     .bx-layout-row{
         @apply w-full flex relative flex-wrap md:flex-nowrap gap-4 lg:gap-6;
     }
+    /* A row whose cells are all empty (say, only members-only blocks on a guest's page) would still take a row gap above and below it, doubling the space between its neighbours. Hide it; a block added later (async loading) brings it back. Studio's builder keeps its empty rows. */
+    body:not(.bx-std) .bx-layout-row:not(:has(.bx-layout-cell > *)) {
+        @apply hidden;
+    }
     .bx-layout-col{
         @apply max-w-full w-full min-w-0;
         flex-shrink: 2 !important;
@@ -1120,8 +1124,9 @@
         @apply bg-white dark:bg-gray-800 shadow;
     }
 
+    /* Popups: a shadow edge and a soft lift (see tailwind.smooth-shadow.js), not a gray ring on top of a shadow. */
     .bx-artificer .bx-popup-border {
-        @apply m-px ring-1 ring-gray-300 dark:ring-gray-700 rounded-2xl;
+        @apply m-px ring-0 smooth-shadow-popover rounded-2xl;
     }
 
     .bx-artificer .bx-popup-width {
@@ -1344,8 +1349,40 @@
     #bx-toolbar .bx-menu-main > .bx-menu-item {
         @apply rounded-full;
     }
+    /* Header main menu: centered between the logo and the toolbar (BxDolMenuMoreAuto keeps it hidden until measured, see artificer menu.css). */
+    #bx-toolbar .bx-menu-main {
+        @apply justify-center;
+    }
     #bx-toolbar .bx-menu-main > .bx-menu-item > .bx-stl-mil {
-        @apply h-11 px-4 py-0 rounded-full;
+        @apply h-11 py-0 ltr:pl-3 ltr:pr-4 rtl:pr-3 rtl:pl-4 rounded-full bx-stl-mil-hover-text; /* the icon's 24px box has ~3px of air, so its side gets 4px less padding to look even */
+    }
+    #bx-toolbar .bx-menu-main > .bx-menu-item-more-auto > .bx-stl-mil {
+        @apply w-11 px-0 justify-center; /* icon-only: a 44px circle like the toolbar buttons */
+    }
+    #bx-toolbar .bx-menu-main .bx-stl-mii:not(:last-child) > * {
+        @apply ltr:mr-2 rtl:ml-2; /* 8px between icon and label in the tabs and the More list */
+    }
+    /*
+     * Header popups (More, account, add content) never run past the bottom of the window: the header is fixed, so the page can't
+     * scroll them into view. The toolbar's own popups (bx-sliding-menu-*) are moved under body when they open, so they are matched
+     * by id rather than by #bx-toolbar.
+     */
+    #bx-toolbar .bx-popup-menu .bx-popup-content,
+    .bx-popup-menu[id^="bx-sliding-menu-"] .bx-popup-content {
+        max-height: calc(100vh - 5.5rem);
+        max-height: calc(100dvh - 5.5rem);
+        @apply overflow-y-auto overscroll-contain;
+    }
+    /* More list: 44px rows like the tabs they continue (same icon and text size), inset 8px so each row's 8px radius sits inside the popup's 16px corner. */
+    #bx-toolbar .bx-popup-menu ul.bx-menu-submenu-more-auto {
+        @apply px-2;
+    }
+    #bx-toolbar ul.bx-menu-submenu-more-auto > .bx-menu-item,
+    #bx-toolbar ul.bx-menu-submenu-more-auto > .bx-menu-item > .bx-stl-mil {
+        @apply rounded-lg;
+    }
+    #bx-toolbar ul.bx-menu-submenu-more-auto > .bx-menu-item > .bx-stl-mil {
+        @apply h-11 py-0 ltr:pl-3 ltr:pr-4 rtl:pr-3 rtl:pl-4 bx-stl-mil-hover-text;
     }
 
     /* Common elements: Page Submenu */

+ 60 - 1
template/scripts/BxBaseMenu.php

@@ -89,7 +89,10 @@ class BxBaseMenu extends BxDolMenu
                 default:
                     trigger_error("Unknown cache mode for menu ({$this->_sObject}): {$this->_aObject['cache']}", E_USER_ERROR);
                     break;
-            }            
+            }
+            // The output marks the selected item (bx-menu-tab-active), so each selected item gets its own entry.
+            $sCacheKey .= $this->_getCacheKeySelected($sCacheKey);
+
             $mixedCache = bx_content_cache_get($sCacheKey);
         }
         if ($mixedCache !== null) {
@@ -288,6 +291,62 @@ class BxBaseMenu extends BxDolMenu
             return $this->_oQuery->getMenuItems();
     }
 
+    /**
+     * Cache key part for the item this menu shows as selected: empty when none of its items is,
+     * so pages that select nothing in this menu share one entry.
+     * The items' module and name pairs are kept in the content cache next to the menu, under the
+     * same key, so a cache hit needs no query and they go stale exactly when the cached menu does.
+     * @param $sCacheKey the menu's own cache key
+     */
+    protected function _getCacheKeySelected ($sCacheKey)
+    {
+        $sItemsKey = $sCacheKey . '_items';
+        $aItems = bx_content_cache_get($sItemsKey);
+        if (!is_array($aItems)) {
+            if (!isset($this->_aObject['menu_items'])) {
+                $this->_aObject['menu_items'] = $this->getMenuItemsRaw();
+            }
+
+            $aItems = $this->_getSelectableItems($this->_aObject['menu_items']);
+            bx_content_cache_set($sItemsKey, $aItems);
+        }
+
+        $sResult = '';
+        foreach ($aItems as $a) {
+            if ($this->_isSelected($a)) {
+                // A short fixed-length suffix whatever the name's length, so the cache file name stays valid.
+                $sResult = '_sel' . substr(hash('sha256', $a['module'] . ':' . $a['name']), 0, 16);
+                break;
+            }
+        }
+
+        return $sResult;
+    }
+
+    /**
+     * Module and name of every item, sub-items included, which is all _isSelected() looks at.
+     */
+    protected function _getSelectableItems ($aItems)
+    {
+        $aResult = [];
+        if (!is_array($aItems)) {
+            return $aResult;
+        }
+
+        foreach ($aItems as $a) {
+            if (!is_array($a) || !isset($a['name'])) {
+                continue;
+            }
+
+            $aResult[] = ['module' => isset($a['module']) ? $a['module'] : '', 'name' => $a['name']];
+            if (!empty($a['subitems'])) {
+                $aResult = array_merge($aResult, $this->_getSelectableItems($a['subitems']));
+            }
+        }
+
+        return $aResult;
+    }
+
     protected function getMenuItemsRawProfileRelated ()
     {
         $oProfile = BxDolProfile::getInstance();

+ 83 - 0
tests/units/template/scripts/BxBaseMenuCacheTest.php

@@ -0,0 +1,83 @@
+<?php
+
+/**
+ * Menus with content cache ('global', 'guest', 'per_user', 'per_acl') keep their rendered HTML, which marks the selected
+ * item with bx-menu-tab-active. The cache key includes the selected item, so the page that happens to render a menu
+ * first doesn't decide the selected item for every other page.
+ */
+class BxBaseMenuCacheTest extends BxDolTestCase
+{
+    private const KEY = 'menu_sys_site_unit_test';
+
+    protected function tearDown(): void
+    {
+        BxDolMenu::setSelectedGlobal('', '');
+        parent::tearDown();
+    }
+
+    private function callProtected($oObject, string $sMethod, array $aArgs = [])
+    {
+        $oMethod = new ReflectionMethod($oObject, $sMethod);
+        return $oMethod->invokeArgs($oObject, $aArgs);
+    }
+
+    private function getSiteMenu(): BxBaseMenu
+    {
+        $oMenu = BxDolMenu::getObjectInstance('sys_site');
+        if (!$oMenu)
+            $this->markTestSkipped('The site menu is not installed.');
+
+        return $oMenu;
+    }
+
+    public function testCacheKeyFollowsTheSelectedItem(): void
+    {
+        $oMenu = $this->getSiteMenu();
+
+        BxDolMenu::setSelectedGlobal('system', 'about');
+        $sAbout = $this->callProtected($oMenu, '_getCacheKeySelected', [self::KEY]);
+
+        BxDolMenu::setSelectedGlobal('system', 'home');
+        $sHome = $this->callProtected($oMenu, '_getCacheKeySelected', [self::KEY]);
+
+        BxDolMenu::setSelectedGlobal('system', 'no-such-item-in-this-menu');
+        $sNone = $this->callProtected($oMenu, '_getCacheKeySelected', [self::KEY]);
+
+        $this->assertNotSame('', $sAbout);
+        $this->assertNotSame('', $sHome);
+        $this->assertNotSame($sAbout, $sHome, 'Different selected items get different cache entries.');
+        $this->assertSame('', $sNone, 'Pages that select nothing in this menu share one entry.');
+        $this->assertSame(strlen($sAbout), strlen($sHome), 'The suffix has a fixed length, whatever the name.');
+        $this->assertLessThanOrEqual(24, strlen($sAbout));
+    }
+
+    public function testItemNamesAreCachedWithTheMenu(): void
+    {
+        $oMenu = $this->getSiteMenu();
+        if (!getParam('sys_content_cache_enable'))
+            $this->markTestSkipped('Content cache is off.');
+
+        BxDolMenu::setSelectedGlobal('system', 'about');
+        $this->callProtected($oMenu, '_getCacheKeySelected', [self::KEY]);
+
+        $aItems = bx_content_cache_get(self::KEY . '_items');
+        $this->assertIsArray($aItems, 'The item list is kept next to the menu, so a cache hit needs no query.');
+        $this->assertContains(['module' => 'system', 'name' => 'about'], $aItems);
+    }
+
+    public function testCachedMenuMarksTheItemOfEachPage(): void
+    {
+        $this->getSiteMenu();
+        if (!getParam('sys_content_cache_enable'))
+            $this->markTestSkipped('Content cache is off.');
+
+        $aActive = [];
+        foreach (['home', 'about', 'home'] as $sName) {
+            BxDolMenu::setSelectedGlobal('system', $sName);
+            preg_match_all('/bx-menu-item bx-menu-item-([\w-]+)[^"]*bx-menu-tab-active/', BxDolMenu::getObjectInstance('sys_site')->getCode(), $aMatches);
+            $aActive[] = implode(',', $aMatches[1]);
+        }
+
+        $this->assertSame(['home', 'about', 'home'], $aActive);
+    }
+}

Some files were not shown because too many files changed in this diff