Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 33 additions & 5 deletions hugo/layouts/partials/header.html
Original file line number Diff line number Diff line change
Expand Up @@ -125,14 +125,42 @@
});
}

function openAt(popover, opener) {
// Issue #1652: on narrow/mobile viewports the shellbar collapses items into
// its built-in overflow (burger) popover. A clicked overflow item reports a
// `targetRef` that is the transient list item INSIDE that popover — and the
// overflow popover closes itself on the same click. A ui5-popover cannot
// anchor to a detached/hidden opener, so opening our nav/share popover against
// that targetRef silently no-ops and the menu just closes. Detect the overflow
// case and anchor to the always-present overflow button instead. Desktop keeps
// anchoring to the specific item button (targetRef).
function isOverflowed(item, targetRef) {
if (item && item.inOverflow === true) return true;
const tag = targetRef && targetRef.tagName && targetRef.tagName.toLowerCase();
return tag === 'ui5-li';
}

function resolveOpener(item, targetRef) {
if (isOverflowed(item, targetRef)) {
return shellbar.shadowRoot?.querySelector('[data-ui5-stable="overflow"]') || shellbar;
}
return targetRef || item;
}

function openAt(popover, opener, deferred) {
closeAllExcept(popover);
if (popover.open) {
popover.open = false;
} else {
return;
}
// When opened from the overflow menu, defer one frame so our popover lands
// AFTER the overflow popover has finished closing — otherwise the two
// popovers fight over focus and ours is dismissed immediately.
const show = () => {
popover.opener = opener;
popover.open = true;
}
};
if (deferred) requestAnimationFrame(show);
else show();
}

shellbar.querySelectorAll('ui5-shellbar-item').forEach(item => {
Expand All @@ -147,11 +175,11 @@
if (window.joule && typeof window.joule.open === 'function') window.joule.open();
}
{{ end }}
else if (id === 'sb-nav') openAt(navPopover, target);
else if (id === 'sb-nav') openAt(navPopover, resolveOpener(item, target), isOverflowed(item, target));
else if (id === 'sb-share') {
shareUrlInput.value = window.location.href;
shareStatus.textContent = '';
openAt(sharePopover, target);
openAt(sharePopover, resolveOpener(item, target), isOverflowed(item, target));
}
else if (id === 'sb-help') window.open('https://community.sap.com', '_blank');
else if (id === 'sb-theme') toggleTheme();
Expand Down
97 changes: 97 additions & 0 deletions test/e2e/header-mobile-nav.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
// e2e: mobile "Navigate" menu (issue #1652).
//
// On narrow/mobile viewports the ui5-shellbar collapses the "Navigate" item
// (#sb-nav) into its built-in overflow (burger) popover. Before the fix, tapping
// "Navigate" inside that overflow menu closed the menu but never opened the
// navigation popover — the clicked item's `targetRef` was the transient list
// item inside the overflow popover, which closes on the same click, and a
// ui5-popover cannot anchor to a detached opener. The fix anchors the nav
// popover to the always-present overflow button and defers the open one frame.
//
// This spec drives the REAL click path at a phone viewport and asserts the nav
// popover (#sb-nav-popover) actually opens.
//
// Self-skips cleanly when SMOKE_BASE_URL / PLAYWRIGHT_BASE_URL is absent so that
// `npm test` (unit suite) is never affected. Runs anonymously — the homepage
// shellbar needs no auth.
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import { hasBaseUrl } from './e2e.config.js';
import { launchBrowser, newPage } from './_browser.js';

const PHONE = { width: 390, height: 780 };

describe.skipIf(!hasBaseUrl())('e2e: mobile Navigate menu (#1652)', () => {
let browser;
beforeAll(async () => {
browser = await launchBrowser();
});
afterAll(async () => {
await browser?.close();
});

it('tapping "Navigate" in the overflow menu opens the navigation popover', async () => {
const { context, page } = await newPage(browser, { authenticated: false });
try {
await page.setViewportSize(PHONE);

const response = await page.goto('/', { waitUntil: 'domcontentloaded' });
expect(response?.status(), 'homepage must return 200').toBe(200);

// Wait for the shellbar custom element + our #sb-nav item to be upgraded.
await page.waitForFunction(
() => !!(customElements.get('ui5-shellbar') && document.getElementById('sb-nav')),
{ timeout: 15_000 },
);

// The overflow layout runs after the shellbar sizes itself. At a phone
// width "Navigate" must land in the overflow menu. If for some reason it
// did not overflow on this env, there is nothing to reproduce — skip
// rather than fail (keeps the suite green on unexpected wide viewports).
const overflowed = await page
.waitForFunction(() => document.getElementById('sb-nav')?.inOverflow === true, {
timeout: 8_000,
})
.then(() => true)
.catch(() => false);
if (!overflowed) {
console.warn('[header-mobile-nav e2e] #sb-nav did not overflow at 390px — skipping');
return;
}

// The nav popover must start closed.
expect(
await page.evaluate(() => document.getElementById('sb-nav-popover')?.open === true),
'nav popover should be closed initially',
).toBe(false);

// 1) Open the shellbar overflow (burger) menu via its real button.
await page.evaluate(() => {
const sb = document.getElementById('app-shellbar');
const btn = sb?.shadowRoot?.querySelector('[data-ui5-stable="overflow"]');
btn?.click();
});

// The #sb-nav item is slotted into the (now open) overflow popover.
await page.locator('#sb-nav').waitFor({ state: 'visible', timeout: 8_000 });

// 2) Tap "Navigate".
await page.locator('#sb-nav').click();

// 3) The navigation popover must open (deferred one frame by the fix).
await page.waitForFunction(
() => document.getElementById('sb-nav-popover')?.open === true,
{ timeout: 5_000 },
);

// And it must actually show the navigation entries.
const navItems = page.locator('#sb-nav-popover ui5-li');
expect(await navItems.count(), 'nav popover should contain nav entries').toBeGreaterThan(0);
expect(
await navItems.first().isVisible(),
'first nav entry should be visible',
).toBe(true);
} finally {
await context.close();
}
});
});
43 changes: 43 additions & 0 deletions test/unit/hugo/header-mobile-nav-overflow.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// test/unit/hugo/header-mobile-nav-overflow.test.ts
//
// Issue #1652: on mobile the shellbar collapses "Navigate"/"Share" into its
// overflow popover. A clicked overflow item's `targetRef` is the transient list
// item inside that popover (which closes on the same click), so anchoring our
// nav/share popover to it silently no-ops. The header script must instead detect
// the overflow case and anchor to the always-present overflow button, deferring
// the open one frame. Cheap regex pins so a future edit can't silently revert
// the fix — the real behavioral coverage lives in test/e2e/header-mobile-nav.test.js.

import { describe, it, expect } from 'vitest'
import { readFileSync } from 'node:fs'
import path from 'node:path'

const header = readFileSync(
path.resolve(import.meta.dirname, '../../../hugo/layouts/partials/header.html'),
'utf8',
)

describe('header.html — mobile overflow nav (#1652)', () => {
it('anchors overflow opens to the shellbar overflow button', () => {
expect(
header.includes('[data-ui5-stable="overflow"]'),
'expected an overflow-button opener lookup',
).toBe(true)
})

it('detects the overflow state before choosing an opener', () => {
// Either the ShellBarItem.inOverflow signal or the overflow list-item tag.
expect(header).toMatch(/inOverflow|isOverflowed/)
})

it('defers the popover open one frame when opened from overflow', () => {
expect(header.includes('requestAnimationFrame'), 'expected a deferred open').toBe(true)
})

it('both the Navigate and Share items use the overflow-aware opener', () => {
// openAt(...) must be called with resolveOpener(...) — not the bare targetRef —
// for both sb-nav and sb-share.
const openCalls = header.match(/openAt\([^)]*resolveOpener\(/g) || []
expect(openCalls.length, 'both nav and share should use resolveOpener').toBeGreaterThanOrEqual(2)
})
})
Loading