From 58c8238167cf7543c04aa4ef311740d457ac3e5e Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Thu, 6 Aug 2026 19:18:16 +0200 Subject: [PATCH 1/8] feat(notes): expose file versions in the note sidebar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Notes have been versioned all along — they are ordinary files, so files_versions keeps history for them without the app doing anything. There was just no way to see it from Notes. Most of the wiring already existed: * PageController dispatches OCA\Files\Event\LoadSidebar, and files_versions registers a listener on that event which adds its sidebar-tab script. The Versions tab has therefore been registered on every Notes page already, simply never rendered. * NotePlain and NoteRich both already subscribe to files_versions:restore:requested and :restored, showing a loading state and refreshing the note afterwards. The restore path was built and unreachable. * The note sidebar already knew how to mount a registered Files sidebar tab as a custom element with the node/folder/view props it expects. The only thing missing was that the sidebar hard-filtered the tab registry down to `id === 'sharing'`. It now renders every tab from an allow-list, so Sharing and Versions sit side by side. Details: * Tab selection moved to a pure function in sidebarTabs.js. It is an allow-list rather than "everything registered", because LoadSidebar brings in whatever every installed app registers and a note sidebar should not grow new tabs when an unrelated app is installed. A tab's own enabled() predicate still has the final say — the versions tab hides itself on public shares and for non-files — but it needs a node to judge, so while the node is still loading tabs are kept and filtered again once it arrives, and a predicate that throws drops that tab instead of taking the sidebar down. * Tabs initialise independently, so one failing to define its custom element no longer hides the others; only a total failure is reported. * New event notes:sidebar:open carries a tab id and is what the app itself emits. notes:share:open is kept as a documented alias, so anything else already emitting it keeps working. * "Versions" action added to the note's action menu, next to "Share". That menu lives in the note list row, so it is present in every editor mode rather than only the non-default one. * Sidebar copy no longer says "sharing" now that it hosts two tabs, and the component, its file and its data-cy hook lose the "share" in their names for the same reason. The e2e specs that assert on the hook follow the rename. Assisted-by: Claude Code:claude-opus-5[1m] Co-Authored-By: Andy Scherzinger Signed-off-by: Frank Karlitschek Signed-off-by: Andy Scherzinger --- playwright/e2e/basic.spec.ts | 2 +- playwright/e2e/note-actions.spec.ts | 11 +- playwright/e2e/note-sidebar.spec.ts | 89 +++++++ playwright/e2e/zen-mode.spec.ts | 2 +- playwright/support/note.ts | 7 + src/App.vue | 6 +- src/components/NoteItem.vue | 21 +- src/components/NoteShareSidebar.vue | 266 --------------------- src/components/NoteSidebar.vue | 356 ++++++++++++++++++++++++++++ src/sidebarTabs.js | 51 ++++ 10 files changed, 530 insertions(+), 281 deletions(-) create mode 100644 playwright/e2e/note-sidebar.spec.ts delete mode 100644 src/components/NoteShareSidebar.vue create mode 100644 src/components/NoteSidebar.vue create mode 100644 src/sidebarTabs.js diff --git a/playwright/e2e/basic.spec.ts b/playwright/e2e/basic.spec.ts index 8a49dbe4f..85ec8ae92 100644 --- a/playwright/e2e/basic.spec.ts +++ b/playwright/e2e/basic.spec.ts @@ -47,7 +47,7 @@ test.describe('Basic checks', () => { await noteItem.locator('.action-item__menutoggle').click() await page.getByRole('menuitem', { name: 'Share', exact: true }).click() - await expect(page.locator('[data-cy-notes-share-sidebar]')).toBeVisible({ timeout: 15000 }) + await expect(page.locator('[data-cy-notes-sidebar]')).toBeVisible({ timeout: 15000 }) await expect(page.getByText('Internal shares')).toBeVisible({ timeout: 15000 }) }) diff --git a/playwright/e2e/note-actions.spec.ts b/playwright/e2e/note-actions.spec.ts index c39796919..8c3d5ffee 100644 --- a/playwright/e2e/note-actions.spec.ts +++ b/playwright/e2e/note-actions.spec.ts @@ -3,18 +3,11 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import type { Locator, Page, TestInfo } from '@playwright/test' +import type { TestInfo } from '@playwright/test' import { expect, test } from '@playwright/test' import { login } from '../support/login.ts' -import { createNote, newNoteButton, noteRow, uniqueTitle } from '../support/note.ts' - -async function openNoteActions(page: Page, noteId: number): Promise { - const row = noteRow(page, noteId) - await row.hover() - await row.locator('.action-item__menutoggle').click() - return row -} +import { createNote, newNoteButton, noteRow, openNoteActions, uniqueTitle } from '../support/note.ts' test.describe('Note actions', () => { test.beforeEach(async ({ page }) => { diff --git a/playwright/e2e/note-sidebar.spec.ts b/playwright/e2e/note-sidebar.spec.ts new file mode 100644 index 000000000..c15a6e39d --- /dev/null +++ b/playwright/e2e/note-sidebar.spec.ts @@ -0,0 +1,89 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import type { Locator, Page, TestInfo } from '@playwright/test' + +import { expect, test } from '@playwright/test' +import { login } from '../support/login.ts' +import { createNote, newNoteButton, openNoteActions, uniqueTitle } from '../support/note.ts' + +interface EventBusWindow extends Window { + _nc_event_bus: { + emit: (name: string, payload: unknown) => void + } +} + +function sidebar(page: Page): Locator { + return page.locator('[data-cy-notes-sidebar]') +} + +function tabButton(page: Page, tabId: string): Locator { + return sidebar(page).locator(`#tab-button-${tabId}`) +} + +function versionsList(page: Page): Locator { + return sidebar(page).locator('[data-files-versions-versions-list]') +} + +async function openSidebarFromActions(page: Page, noteId: number, action: string): Promise { + await openNoteActions(page, noteId) + await page.getByRole('menuitem', { name: action, exact: true }).click() + await expect(sidebar(page)).toBeVisible({ timeout: 15000 }) +} + +test.describe('Note sidebar', () => { + test.beforeEach(async ({ page }) => { + await login(page) + await page.goto('/index.php/apps/notes/') + await expect(newNoteButton(page)).toBeVisible() + }) + + test('opens the versions tab from the actions menu', async ({ page }, testInfo: TestInfo) => { + const noteId = await createNote(page, uniqueTitle('versions', testInfo)) + + await openSidebarFromActions(page, noteId, 'Versions') + + await expect(tabButton(page, 'files_versions')).toHaveAttribute('aria-selected', 'true') + await expect(versionsList(page)).toBeAttached({ timeout: 15000 }) + }) + + test('renders the allow-listed tabs only', async ({ page }, testInfo: TestInfo) => { + const noteId = await createNote(page, uniqueTitle('sidebar-tabs', testInfo)) + + await openSidebarFromActions(page, noteId, 'Share') + + await expect(tabButton(page, 'sharing')).toBeVisible() + await expect(tabButton(page, 'files_versions')).toBeVisible() + await expect(sidebar(page).getByRole('tab')).toHaveCount(2) + }) + + test('switches between the sharing and versions tabs', async ({ page }, testInfo: TestInfo) => { + const noteId = await createNote(page, uniqueTitle('sidebar-switch', testInfo)) + + await openSidebarFromActions(page, noteId, 'Share') + await expect(page.getByText('Internal shares')).toBeVisible({ timeout: 15000 }) + + await tabButton(page, 'files_versions').click() + await expect(tabButton(page, 'files_versions')).toHaveAttribute('aria-selected', 'true') + await expect(versionsList(page)).toBeAttached({ timeout: 15000 }) + + await tabButton(page, 'sharing').click() + await expect(tabButton(page, 'sharing')).toHaveAttribute('aria-selected', 'true') + await expect(page.getByText('Internal shares')).toBeVisible() + }) + + test('falls back to the first tab when the requested one is unavailable', async ({ page }, testInfo: TestInfo) => { + const noteId = await createNote(page, uniqueTitle('sidebar-fallback', testInfo)) + + await page.evaluate((id) => { + (window as unknown as EventBusWindow)._nc_event_bus + .emit('notes:sidebar:open', { noteId: id, tab: 'not-a-note-sidebar-tab' }) + }, noteId) + + await expect(sidebar(page)).toBeVisible({ timeout: 15000 }) + await expect(tabButton(page, 'sharing')).toHaveAttribute('aria-selected', 'true') + await expect(page.getByText('Internal shares')).toBeVisible({ timeout: 15000 }) + }) +}) diff --git a/playwright/e2e/zen-mode.spec.ts b/playwright/e2e/zen-mode.spec.ts index 4a777b5ca..58b9a295e 100644 --- a/playwright/e2e/zen-mode.spec.ts +++ b/playwright/e2e/zen-mode.spec.ts @@ -38,7 +38,7 @@ function shareButton(page: Page): Locator { } function shareSidebar(page: Page): Locator { - return page.locator('[data-cy-notes-share-sidebar]') + return page.locator('[data-cy-notes-sidebar]') } async function expectZenMode(page: Page, active: boolean): Promise { diff --git a/playwright/support/note.ts b/playwright/support/note.ts index 83eb612a6..ae0ef1faf 100644 --- a/playwright/support/note.ts +++ b/playwright/support/note.ts @@ -26,6 +26,13 @@ export function noteRow(page: Page, noteId: number): Locator { .locator('xpath=ancestor::li[1]') } +export async function openNoteActions(page: Page, noteId: number): Promise { + const row = noteRow(page, noteId) + await row.hover() + await row.locator('.action-item__menutoggle').click() + return row +} + export async function waitForNoteRoute(page: Page, previousNoteId: number | null): Promise { await expect.poll(() => currentNoteId(page)).not.toBe(previousNoteId) diff --git a/src/App.vue b/src/App.vue index f94e85728..2f848a0b3 100644 --- a/src/App.vue +++ b/src/App.vue @@ -69,7 +69,7 @@ - + @@ -89,7 +89,7 @@ import ShareVariantOutlineIcon from 'vue-material-design-icons/ShareVariantOutli import AppSettings from './components/AppSettings.vue' import CategoriesList from './components/CategoriesList.vue' import EditorHint from './components/Modal/EditorHint.vue' -import NoteShareSidebar from './components/NoteShareSidebar.vue' +import NoteSidebar from './components/NoteSidebar.vue' import { config } from './config.js' import logger from './Logger.js' import { fetchNotes, noteExists, undoDeleteNote } from './NotesService.js' @@ -116,7 +116,7 @@ export default { NcButton, NcContent, FocusIcon, - NoteShareSidebar, + NoteSidebar, ShareVariantOutlineIcon, }, diff --git a/src/components/NoteItem.vue b/src/components/NoteItem.vue index 204dba61d..e1940645d 100644 --- a/src/components/NoteItem.vue +++ b/src/components/NoteItem.vue @@ -42,6 +42,13 @@ {{ t('notes', 'Share') }} + + + {{ t('notes', 'Versions') }} + + {{ fullscreen ? t('notes', 'Exit full screen') : t('notes', 'Full screen') }} + + + {{ t('notes', 'Open sidebar') }} + @@ -112,6 +118,7 @@ import NcActionButton from '@nextcloud/vue/components/NcActionButton' import NcActions from '@nextcloud/vue/components/NcActions' import NcAppContent from '@nextcloud/vue/components/NcAppContent' import NcModal from '@nextcloud/vue/components/NcModal' +import DockRightIcon from 'vue-material-design-icons/DockRight.vue' import EyeOutlineIcon from 'vue-material-design-icons/EyeOutline.vue' import FullscreenIcon from 'vue-material-design-icons/Fullscreen.vue' import PencilOffOutlineIcon from 'vue-material-design-icons/PencilOffOutline.vue' @@ -131,6 +138,7 @@ export default { components: { ConflictSolution, + DockRightIcon, PencilOutlineIcon, EyeOutlineIcon, FullscreenIcon, @@ -268,6 +276,11 @@ export default { this.actionsOpen = false }, + onOpenSidebar() { + this.actionsOpen = false + emit('notes:sidebar:open', { noteId: this.noteId }) + }, + onDetectFullscreen() { this.fullscreen = document.fullScreen || document.mozFullScreen || document.webkitIsFullScreen }, From 9efd1b08b04a148cb223e45a078cf63333cd1808 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Sun, 16 Aug 2026 15:51:40 +0200 Subject: [PATCH 3/8] feat(notes): show size, modification date and owner in the note sidebar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sidebar header carried only the note title, while the Files sidebar shows the file metadata right below it. Reimplements the subname the Files app renders in its sidebar header (apps/files/src/components/FilesSidebar/FilesSidebarSubname.vue): the formatted file size, the modification date and the owner as a user bubble with avatar and display name, rendered through NcAppSidebar's subname slot. No new data plumbing was needed — fetchDavNode() already uses the default propfind, which asks for getcontentlength, getlastmodified, owner-id and owner-display-name. The metadata is the state of the file at the moment the sidebar was opened; nothing re-fetches the node while it stays open. Assisted-by: Claude Code:claude-opus-5[1m] Signed-off-by: Andy Scherzinger --- playwright/e2e/note-sidebar.spec.ts | 15 ++++++ src/components/NoteSidebar.vue | 6 +++ src/components/NoteSidebarSubname.vue | 74 +++++++++++++++++++++++++++ 3 files changed, 95 insertions(+) create mode 100644 src/components/NoteSidebarSubname.vue diff --git a/playwright/e2e/note-sidebar.spec.ts b/playwright/e2e/note-sidebar.spec.ts index 7b9b68a41..7ba6f7b2c 100644 --- a/playwright/e2e/note-sidebar.spec.ts +++ b/playwright/e2e/note-sidebar.spec.ts @@ -27,6 +27,10 @@ function versionsList(page: Page): Locator { return sidebar(page).locator('[data-files-versions-versions-list]') } +function subname(page: Page): Locator { + return sidebar(page).locator('.app-sidebar-header__subname') +} + async function openSidebarFromActions(page: Page, noteId: number, action: string): Promise { await openNoteActions(page, noteId) await page.getByRole('menuitem', { name: action, exact: true }).click() @@ -49,6 +53,17 @@ test.describe('Note sidebar', () => { await expect(versionsList(page)).toBeAttached({ timeout: 15000 }) }) + test('shows the size, the modification date and the owner of the note', async ({ page }, testInfo: TestInfo) => { + const noteId = await createNote(page, uniqueTitle('sidebar-subname', testInfo)) + + await openSidebarFromActions(page, noteId, 'Share') + + await expect(subname(page)).toBeVisible({ timeout: 15000 }) + await expect(subname(page)).toContainText(/\d+(\.\d+)?\s?(B|KB|MB|GB)/) + await expect(subname(page).locator('[data-timestamp]')).toBeVisible() + await expect(subname(page).locator('.user-bubble__content')).toContainText('admin') + }) + test('renders the allow-listed tabs only', async ({ page }, testInfo: TestInfo) => { const noteId = await createNote(page, uniqueTitle('sidebar-tabs', testInfo)) diff --git a/src/components/NoteSidebar.vue b/src/components/NoteSidebar.vue index efe888b8c..2bee6ae90 100644 --- a/src/components/NoteSidebar.vue +++ b/src/components/NoteSidebar.vue @@ -15,6 +15,10 @@ @closed="onClosed" @update:open="onToggle" > + + + + + + + + From 87b26fca4cb855272950c0f49cfc87c7ec8eb588 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Sun, 16 Aug 2026 17:27:47 +0200 Subject: [PATCH 4/8] refactor(notes): simplify the note sidebar state handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sidebar kept three collections to answer one question — is this tab's custom element defined? * initializedTabs was only ever written once customElements.whenDefined() had resolved, which is exactly when customElements.get() starts returning the constructor — and that is checked first in the same guard. Redundant, so it is gone. * pendingTabs caches an in-flight side effect on the global custom element registry rather than component state, so it moves out of the component into sidebarTabs.js, along with the two functions that use it and the timeout they share, where they can be unit tested. * loadingTab is raised once per run instead of from the per-tab helper. Guarding it per tab bought nothing, since loadNodeContext() raises loadingContext on every open anyway. The active tab is now clamped by a computed feeding the `active` prop, with a watcher writing the clamped id back so the two-way bound activeTab does not keep naming a tab that is not on screen. The reset shared by onSidebarOpen and onClosed became a single method. tabError is dropped as well: now that a tab failing to define its element is no longer offered, it could only be set when no tab was left at all, where the empty state already carries a message. The failure itself is still logged with the tab id. Assisted-by: Claude Code:claude-opus-5[1m] Signed-off-by: Andy Scherzinger --- src/components/NoteSidebar.vue | 148 +++++++++------------------------ src/sidebarTabs.js | 66 +++++++++++++++ 2 files changed, 105 insertions(+), 109 deletions(-) diff --git a/src/components/NoteSidebar.vue b/src/components/NoteSidebar.vue index 2bee6ae90..1e5880bb3 100644 --- a/src/components/NoteSidebar.vue +++ b/src/components/NoteSidebar.vue @@ -45,7 +45,7 @@ - {{ tabError || t('notes', 'Sharing and versions are not available right now.') }} + {{ t('notes', 'Sharing and versions are not available right now.') }} @@ -72,15 +72,10 @@ import NcLoadingIcon from '@nextcloud/vue/components/NcLoadingIcon' import FileOutlineIcon from 'vue-material-design-icons/FileOutline.vue' import NoteSidebarSubname from './NoteSidebarSubname.vue' import logger from '../Logger.js' -import { selectNoteSidebarTabs } from '../sidebarTabs.js' +import { initializeSidebarTab, selectNoteSidebarTabs } from '../sidebarTabs.js' import store from '../store.js' import { fetchDavNode } from '../WebdavService.js' -// customElements.whenDefined() never settles for an element that is never -// defined, so a tab whose onInit() does not deliver one must not be waited for -// forever -const TAB_DEFINITION_TIMEOUT = 10000 - export default { name: 'NoteSidebar', @@ -101,14 +96,11 @@ export default { contextRequestToken: 0, currentFolder: null, currentNode: null, - pendingTabs: new Map(), - initializedTabs: new Set(), failedTabs: new Set(), isOpen: false, loadingContext: false, loadingTab: false, noteId: null, - tabError: '', } }, @@ -136,6 +128,20 @@ export default { return this.availableTabs.filter((tab) => !this.failedTabs.has(tab.tagName)) }, + /** + * NcAppSidebar falls back to its first tab when the active one is not + * among them, but does not report that back, so the tab id has to be + * clamped here as well for `active` to reach the right custom element. + * The watcher below writes the clamped id back, so `activeTab` does not + * keep naming a tab that is not on screen. + */ + resolvedTab() { + if (this.tabs.some(({ id }) => id === this.activeTab)) { + return this.activeTab + } + return this.tabs[0]?.id ?? this.activeTab + }, + currentView() { return { id: 'notes', @@ -145,10 +151,10 @@ export default { }, watch: { - // the versions tab drops out once the node says it is not applicable, - // so what was requested is not necessarily still renderable - tabs(tabs) { - this.activeTab = this.resolveTab(this.activeTab, tabs) + resolvedTab(tabId) { + if (tabId !== this.activeTab) { + this.activeTab = tabId + } }, }, @@ -173,78 +179,30 @@ export default { } const requestToken = this.contextRequestToken + this.loadingTab = true - // One tab failing to define its element must not hide the others, so - // they are initialised independently and only a total failure is - // reported as an error. - const results = await Promise.all(tabs.map((tab) => this.initializeTab(tab))) + const results = await Promise.all(tabs.map(initializeSidebarTab)) if (requestToken !== this.contextRequestToken) { return } - this.loadingTab = false - this.tabError = results.includes(true) - ? '' - : this.t('notes', 'Failed to load the note sidebar.') - }, - - /** - * @param {object} tab a registered Files sidebar tab - * @return {Promise} whether the tab is usable - */ - async initializeTab(tab) { - if (window.customElements.get(tab.tagName) || this.initializedTabs.has(tab.tagName)) { - return true - } - - this.loadingTab = true - - // an open while another one is still initializing the same element - // has to await that initialization, not assume it succeeded - const pending = this.pendingTabs.get(tab.tagName) - if (pending) { - return pending - } - - const initialization = this.defineTabElement(tab) - this.pendingTabs.set(tab.tagName, initialization) + tabs.forEach((tab, index) => { + if (!results[index]) { + this.failedTabs.add(tab.tagName) + } + }) - try { - return await initialization - } finally { - this.pendingTabs.delete(tab.tagName) - } + this.loadingTab = false }, - /** - * @param {object} tab a registered Files sidebar tab - * @return {Promise} whether its custom element got defined - */ - async defineTabElement(tab) { - let timeout - try { - await Promise.race([ - (async () => { - await tab.onInit?.() - await window.customElements.whenDefined(tab.tagName) - })(), - new Promise((resolve, reject) => { - timeout = setTimeout( - () => reject(new Error(`${tab.tagName} was not defined in time`)), - TAB_DEFINITION_TIMEOUT, - ) - }), - ]) - this.initializedTabs.add(tab.tagName) - return true - } catch (error) { - logger.error('Failed to initialize a sidebar tab in Notes', { error, tab: tab.id }) - this.failedTabs.add(tab.tagName) - return false - } finally { - clearTimeout(timeout) - } + resetContext() { + this.contextRequestToken += 1 + this.contextError = '' + this.currentNode = null + this.currentFolder = null + this.loadingContext = false + this.loadingTab = false }, async loadNodeContext() { @@ -293,38 +251,16 @@ export default { } }, - /** - * NcAppSidebar falls back to its first tab when the active one is not - * among them, but does not report that back, so the tab id here has to - * be clamped as well for `active` to reach the right custom element. - * - * @param {string} tab the requested tab id - * @param {Array} tabs the tabs currently rendered - * @return {string} the requested tab if renderable, the first one otherwise - */ - resolveTab(tab, tabs) { - if (tabs.length === 0 || tabs.some(({ id }) => id === tab)) { - return tab - } - return tabs[0].id - }, - onShareOpen({ noteId }) { return this.onSidebarOpen({ noteId, tab: 'sharing' }) }, async onSidebarOpen({ noteId, tab = 'sharing' }) { - this.contextRequestToken += 1 + this.resetContext() this.noteId = Number(noteId) this.isOpen = true - this.contextError = '' - this.tabError = '' - this.currentNode = null - this.currentFolder = null - this.loadingContext = false - this.loadingTab = false this.failedTabs.clear() - this.activeTab = this.resolveTab(tab, this.tabs) + this.activeTab = tab if (this.availableTabs.length === 0) { await this.initializeTabs() @@ -348,14 +284,8 @@ export default { return } - this.contextRequestToken += 1 + this.resetContext() this.noteId = null - this.contextError = '' - this.currentNode = null - this.currentFolder = null - this.loadingContext = false - this.loadingTab = false - this.tabError = '' }, }, } diff --git a/src/sidebarTabs.js b/src/sidebarTabs.js index 78c615ff6..43369729a 100644 --- a/src/sidebarTabs.js +++ b/src/sidebarTabs.js @@ -49,3 +49,69 @@ export function selectNoteSidebarTabs(tabs, { node = null, folder = null, view = }) .sort((a, b) => (a.order ?? 0) - (b.order ?? 0)) } + +/** + * How long a tab is given to define its custom element. + * + * customElements.whenDefined() never settles for an element that is never + * defined, so a tab whose onInit() does not deliver one must not be waited for + * forever. + * + * @type {number} + */ +export const TAB_DEFINITION_TIMEOUT = 10000 + +/** Initializations in flight, keyed by tag name, so opens can share one. */ +const pendingTabs = new Map() + +/** + * @param {object} tab a registered Files sidebar tab + * @return {Promise} whether its custom element got defined + */ +async function defineTabElement(tab) { + let timeout + try { + await Promise.race([ + (async () => { + await tab.onInit?.() + await window.customElements.whenDefined(tab.tagName) + })(), + new Promise((resolve, reject) => { + timeout = setTimeout( + () => reject(new Error(`${tab.tagName} was not defined in time`)), + TAB_DEFINITION_TIMEOUT, + ) + }), + ]) + return true + } catch (error) { + logger.error('Failed to initialize a sidebar tab in Notes', { error, tab: tab.id }) + return false + } finally { + clearTimeout(timeout) + } +} + +/** + * Bring a tab's custom element into the registry, once per element at a time. + * + * An open while another one is still defining the same element awaits that + * initialization rather than assuming it succeeded. + * + * @param {object} tab a registered Files sidebar tab + * @return {Promise} whether the tab is usable + */ +export function initializeSidebarTab(tab) { + if (window.customElements.get(tab.tagName)) { + return Promise.resolve(true) + } + + if (!pendingTabs.has(tab.tagName)) { + pendingTabs.set( + tab.tagName, + defineTabElement(tab).finally(() => pendingTabs.delete(tab.tagName)), + ) + } + + return pendingTabs.get(tab.tagName) +} From 99370ecb8725bdea38b5edf6295f948af9376897 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Sun, 16 Aug 2026 17:49:12 +0200 Subject: [PATCH 5/8] feat(notes): outline the sharing tab icon until its tab is active MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sidebar tabs should carry outlined icons that fill once the tab is active. The sharing tab now renders ShareVariantOutline while inactive and ShareVariant while active, following the pattern from nextcloud/tables#2672. The switch happens inside the #icon slot rather than through a dedicated slot, as @nextcloud/vue has no #icon-active yet: NcAppSidebarTab exposes renderIcon() without arguments. That is enough here, because the tab button invokes renderIcon() from its own render function, so reading the resolved tab id there tracks it. Only the sharing tab is overridden. Every other tab keeps the icon its app registered, versions included — there is no outlined counterpart of the backup-restore icon to fill in. Mixing the two icon systems misaligns the nav: NcIconSvgWrapper reserves a clickable-area box around its svg while a material design icon is only as big as itself, which left the versions icon 7px below the sharing one. The wrapper's inline modifier drops that box. Assisted-by: Claude Code:claude-opus-5[1m] Signed-off-by: Andy Scherzinger --- playwright/e2e/note-sidebar.spec.ts | 33 +++++++++++++++++++++++++++++ src/components/NoteSidebar.vue | 10 ++++++++- 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/playwright/e2e/note-sidebar.spec.ts b/playwright/e2e/note-sidebar.spec.ts index 7ba6f7b2c..5d248e8ef 100644 --- a/playwright/e2e/note-sidebar.spec.ts +++ b/playwright/e2e/note-sidebar.spec.ts @@ -89,6 +89,39 @@ test.describe('Note sidebar', () => { await expect(page.getByText('Internal shares')).toBeVisible() }) + test('fills the sharing icon only while its tab is active', async ({ page }, testInfo: TestInfo) => { + const noteId = await createNote(page, uniqueTitle('sidebar-icons', testInfo)) + + await openSidebarFromActions(page, noteId, 'Share') + + await expect(tabButton(page, 'sharing').locator('.share-variant-icon')).toBeVisible() + await expect(tabButton(page, 'sharing').locator('.share-variant-outline-icon')).toHaveCount(0) + + await tabButton(page, 'files_versions').click() + + await expect(tabButton(page, 'sharing').locator('.share-variant-outline-icon')).toBeVisible() + await expect(tabButton(page, 'sharing').locator('.share-variant-icon')).toHaveCount(0) + }) + + test('lines the tab icons up with each other', async ({ page }, testInfo: TestInfo) => { + const noteId = await createNote(page, uniqueTitle('sidebar-align', testInfo)) + + await openSidebarFromActions(page, noteId, 'Share') + await expect(tabButton(page, 'files_versions')).toBeVisible() + + const icons = await page.evaluate(() => { + const box = (id: string) => { + const selector = `#tab-button-${id} :is(.icon-vue, .material-design-icon)` + const { y, height } = document.querySelector(selector)!.getBoundingClientRect() + return { y, height } + } + return { sharing: box('sharing'), versions: box('files_versions') } + }) + + expect(icons.versions.y).toBeCloseTo(icons.sharing.y, 0) + expect(icons.versions.height).toBeCloseTo(icons.sharing.height, 0) + }) + test('falls back to the first tab when the requested one is unavailable', async ({ page }, testInfo: TestInfo) => { const noteId = await createNote(page, uniqueTitle('sidebar-fallback', testInfo)) diff --git a/src/components/NoteSidebar.vue b/src/components/NoteSidebar.vue index 1e5880bb3..9112ccbd2 100644 --- a/src/components/NoteSidebar.vue +++ b/src/components/NoteSidebar.vue @@ -26,7 +26,11 @@ :order="tab.order" > @@ -70,6 +74,8 @@ import NcEmptyContent from '@nextcloud/vue/components/NcEmptyContent' import NcIconSvgWrapper from '@nextcloud/vue/components/NcIconSvgWrapper' import NcLoadingIcon from '@nextcloud/vue/components/NcLoadingIcon' import FileOutlineIcon from 'vue-material-design-icons/FileOutline.vue' +import ShareVariantIcon from 'vue-material-design-icons/ShareVariant.vue' +import ShareVariantOutlineIcon from 'vue-material-design-icons/ShareVariantOutline.vue' import NoteSidebarSubname from './NoteSidebarSubname.vue' import logger from '../Logger.js' import { initializeSidebarTab, selectNoteSidebarTabs } from '../sidebarTabs.js' @@ -87,6 +93,8 @@ export default { NcLoadingIcon, FileOutlineIcon, NoteSidebarSubname, + ShareVariantIcon, + ShareVariantOutlineIcon, }, data() { From 73bea3b3698ecf7ab0f8a762f3e00a6020c51ca6 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Sun, 16 Aug 2026 21:35:38 +0200 Subject: [PATCH 6/8] fix(notes): refresh the sidebar versions list after a restore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restoring an older version left the list showing the state from when the sidebar was opened. It took a reload or reopening the sidebar to see the restored version as the current one. The versions tab already reloads itself when the mtime of the node it was handed changes, and emits files:node:updated with a node carrying the restored etag, size and mtime. The Files sidebar closes that loop by swapping its current node whenever such an event names it, which is what the note sidebar now does too — matching on source, as the Files sidebar store does. The subname in the sidebar header picks the update up as well, so size and modification date no longer lag behind a restore either. The test emits the event files_versions sends out after a restore and watches for the reload it triggers, rather than restoring for real: what the sidebar has to do is the same either way, and the outcome then does not hinge on how a server stamps a rollback. Assisted-by: Claude Code:claude-opus-5[1m] Signed-off-by: Andy Scherzinger --- playwright/e2e/note-sidebar.spec.ts | 35 +++++++++++++++++++++++++++++ src/components/NoteSidebar.vue | 15 +++++++++++++ 2 files changed, 50 insertions(+) diff --git a/playwright/e2e/note-sidebar.spec.ts b/playwright/e2e/note-sidebar.spec.ts index 5d248e8ef..14a638042 100644 --- a/playwright/e2e/note-sidebar.spec.ts +++ b/playwright/e2e/note-sidebar.spec.ts @@ -15,6 +15,11 @@ interface EventBusWindow extends Window { } } +interface NodeLike { + mtime: Date + clone: () => NodeLike +} + function sidebar(page: Page): Locator { return page.locator('[data-cy-notes-sidebar]') } @@ -27,6 +32,12 @@ function versionsList(page: Page): Locator { return sidebar(page).locator('[data-files-versions-versions-list]') } +// scoped to the list rather than the sidebar: the sharing tab's element reports +// itself as its own shadow root, which sends a piercing query into a loop +function versionEntries(page: Page): Locator { + return page.locator('[data-files-versions-versions-list] [data-files-versions-version]') +} + function subname(page: Page): Locator { return sidebar(page).locator('.app-sidebar-header__subname') } @@ -53,6 +64,30 @@ test.describe('Note sidebar', () => { await expect(versionsList(page)).toBeAttached({ timeout: 15000 }) }) + test('reloads the versions list when the note is updated', async ({ page }, testInfo: TestInfo) => { + const noteId = await createNote(page, uniqueTitle('sidebar-reload', testInfo)) + + await openSidebarFromActions(page, noteId, 'Versions') + await expect(versionEntries(page).first()).toBeVisible({ timeout: 15000 }) + + let reloads = 0 + page.on('request', (request) => { + if (request.method() === 'PROPFIND' && request.url().includes('/remote.php/dav/versions/')) { + reloads += 1 + } + }) + + // what files_versions hands out once it has restored a version + await page.evaluate(() => { + const tab = document.querySelector('files-versions_sidebar-tab') as unknown as { node: NodeLike } + const node = tab.node.clone() + node.mtime = new Date(node.mtime.getTime() - 60000) + ;(window as unknown as EventBusWindow)._nc_event_bus.emit('files:node:updated', node) + }) + + await expect.poll(() => reloads, { timeout: 15000 }).toBeGreaterThan(0) + }) + test('shows the size, the modification date and the owner of the note', async ({ page }, testInfo: TestInfo) => { const noteId = await createNote(page, uniqueTitle('sidebar-subname', testInfo)) diff --git a/src/components/NoteSidebar.vue b/src/components/NoteSidebar.vue index 9112ccbd2..061f6b93a 100644 --- a/src/components/NoteSidebar.vue +++ b/src/components/NoteSidebar.vue @@ -171,11 +171,13 @@ export default { // the sharing tab; it stays so anything still emitting it keeps working subscribe('notes:share:open', this.onShareOpen) subscribe('notes:sidebar:open', this.onSidebarOpen) + subscribe('files:node:updated', this.onNodeUpdated) }, unmounted() { unsubscribe('notes:share:open', this.onShareOpen) unsubscribe('notes:sidebar:open', this.onSidebarOpen) + unsubscribe('files:node:updated', this.onNodeUpdated) }, methods: { @@ -259,6 +261,19 @@ export default { } }, + /** + * Tabs report what they changed about the note through this event — a + * restored version for instance — and hand out a node of their own, + * which they in turn watch for changes. + * + * @param {object} node the updated node + */ + onNodeUpdated(node) { + if (node?.source && node.source === this.currentNode?.source) { + this.currentNode = node + } + }, + onShareOpen({ noteId }) { return this.onSidebarOpen({ noteId, tab: 'sharing' }) }, From 761ed161a6c33c4e4a284ca14b880ba84343459d Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Sun, 16 Aug 2026 21:36:04 +0200 Subject: [PATCH 7/8] fix(notes): keep the editor behind a spinner while a version is restored MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restoring a version left the editor showing the old content until its periodic refresh came around, with nothing indicating that anything was going on — and that stale content could be typed into meanwhile. Both editors already handled this, but were never reached: they read a payload files_versions no longer emits — a fileInfo key, and a fileId on the version — so the requested handler threw on the missing key and the restored one always returned early. They take the node from the event now and compare its fileid. That brings back the loading state, which replaces the editor with a spinner and thereby keeps it from being typed into while the content is swapped, along with the immediate refresh once the restore lands. Two things were needed for that state to mean anything: NotePlain's refreshNote() returns its promise now, as it would otherwise be cleared before the new content arrived, and both editors clear it on files_versions:restore:failed, which would leave the editor stuck behind the spinner for good. The test delays the restore request so the window it asserts on is not a race, and opens the note explicitly, as a reload would leave the editor on whichever note was open before. Its revisions are written over WebDAV rather than through the app, which would retitle — and thereby rename — the note from its changed content, and they are spaced out because recent versions are thinned to one per two seconds. Version entries are located from the list rather than from the sidebar: the sharing tab's element reports itself as its own shadow root, which sends a piercing query into a loop. The poll that waited for a conflict button to auto-click goes as well. It looked for data-cy="resolveServerVersion", which exists neither in Notes nor in Text — Text's collision dialog offers useEditorVersion and useReaderVersion — so it never hit and never stopped, and fixing the guard above would have turned it into a timer per restore that runs for as long as the page is open. Pressing that button for the user would mean discarding whatever they had typed but not yet saved, which is the very thing the dialog asks about, so the dialog is left to them. Assisted-by: Claude Code:claude-opus-5[1m] Signed-off-by: Andy Scherzinger --- playwright/e2e/note-sidebar.spec.ts | 74 ++++++++++++++++++++++++++++- playwright/support/note.ts | 41 +++++++++++++++- src/components/NotePlain.vue | 51 +++++++++++++------- src/components/NoteRich.vue | 37 +++++++++------ 4 files changed, 168 insertions(+), 35 deletions(-) diff --git a/playwright/e2e/note-sidebar.spec.ts b/playwright/e2e/note-sidebar.spec.ts index 14a638042..cae90f41b 100644 --- a/playwright/e2e/note-sidebar.spec.ts +++ b/playwright/e2e/note-sidebar.spec.ts @@ -7,7 +7,8 @@ import type { Locator, Page, TestInfo } from '@playwright/test' import { expect, test } from '@playwright/test' import { login } from '../support/login.ts' -import { createNote, newNoteButton, openNoteActions, setNoteMode, uniqueTitle } from '../support/note.ts' +import { createNote, createNoteRevisions, newNoteButton, openNoteActions, setNoteMode, uniqueTitle } from '../support/note.ts' +import { NoteEditor } from '../support/sections/NoteEditor.ts' interface EventBusWindow extends Window { _nc_event_bus: { @@ -88,6 +89,77 @@ test.describe('Note sidebar', () => { await expect.poll(() => reloads, { timeout: 15000 }).toBeGreaterThan(0) }) + test('keeps the editor behind a spinner while a restored version loads', async ({ page, request }) => { + const noteId = await createNoteRevisions(request, [ + 'Restore spinner\n\nrevision one', + 'Restore spinner\n\nrevision two', + ]) + // the editor has to hold this note, not whichever one was open before + await page.goto(`/index.php/apps/notes/note/${noteId}`) + + // hold the restore long enough to observe what the editor does meanwhile + await page.route('**/remote.php/dav/versions/**', async (route) => { + if (route.request().method() === 'MOVE') { + await new Promise((resolve) => setTimeout(resolve, 3000)) + } + await route.continue() + }) + + await openSidebarFromActions(page, noteId, 'Versions') + + const entries = versionEntries(page) + await expect(entries.nth(1)).toBeVisible({ timeout: 15000 }) + + const editor = page.locator('.text-editor, .note-editor') + const spinner = page.locator('#app-content-vue.loading, .text-editor-wrapper.loading') + await expect(editor).toBeVisible() + + await entries.last().hover() + await entries.last().locator('.action-item__menutoggle').first().click() + await page.getByRole('menuitem', { name: 'Restore version' }).click() + + // the editor is gone while the restore runs, so it cannot be typed into + await expect(spinner).toBeVisible() + await expect(editor).toBeHidden() + + await expect(editor).toBeVisible({ timeout: 20000 }) + await new NoteEditor(page).expectText('Restore spinner\n\nrevision one') + }) + + test('gives the editor back when a restore fails', async ({ page, request }) => { + const noteId = await createNoteRevisions(request, [ + 'Restore failure\n\nrevision one', + 'Restore failure\n\nrevision two', + ]) + await page.goto(`/index.php/apps/notes/note/${noteId}`) + + await page.route('**/remote.php/dav/versions/**', async (route) => { + if (route.request().method() === 'MOVE') { + await new Promise((resolve) => setTimeout(resolve, 1000)) + await route.fulfill({ status: 500 }) + return + } + await route.continue() + }) + + await openSidebarFromActions(page, noteId, 'Versions') + + const entries = versionEntries(page) + await expect(entries.nth(1)).toBeVisible({ timeout: 15000 }) + + const editor = page.locator('.text-editor, .note-editor') + await expect(editor).toBeVisible() + + await entries.last().hover() + await entries.last().locator('.action-item__menutoggle').first().click() + await page.getByRole('menuitem', { name: 'Restore version' }).click() + + await expect(page.locator('#app-content-vue.loading, .text-editor-wrapper.loading')).toBeVisible() + + // a failed restore must not leave the editor behind the spinner + await expect(editor).toBeVisible({ timeout: 15000 }) + }) + test('shows the size, the modification date and the owner of the note', async ({ page }, testInfo: TestInfo) => { const noteId = await createNote(page, uniqueTitle('sidebar-subname', testInfo)) diff --git a/playwright/support/note.ts b/playwright/support/note.ts index f72c39c2a..351672c2e 100644 --- a/playwright/support/note.ts +++ b/playwright/support/note.ts @@ -12,10 +12,47 @@ export function uniqueTitle(prefix: string, testInfo: TestInfo): string { return `Playwright ${prefix} ${testInfo.parallelIndex}-${Date.now()}` } +function apiUser(): string { + return process.env.NC_USER ?? 'admin' +} + function apiHeaders(): Record { - const user = process.env.NC_USER ?? 'admin' const password = process.env.NC_PASS ?? 'admin' - return { Authorization: `Basic ${Buffer.from(`${user}:${password}`).toString('base64')}` } + return { Authorization: `Basic ${Buffer.from(`${apiUser()}:${password}`).toString('base64')}` } +} + +/** + * Create a note and rewrite it through WebDAV until it has one version per + * given revision. Writing goes around the app on purpose, so the note keeps its + * file name instead of being retitled from the changed content. + * + * @param request The request fixture to use + * @param revisions The contents to write, oldest first + * @return The id of the created note + */ +export async function createNoteRevisions(request: APIRequestContext, revisions: string[]): Promise { + expect(revisions.length, 'revisions to write').toBeGreaterThan(0) + + const created = await request.post('/index.php/apps/notes/api/v1/notes', { + headers: apiHeaders(), + data: { content: revisions[0] }, + }) + expect(created.ok(), 'creating the note').toBeTruthy() + + const note = await created.json() + const path = note.internalPath.split('/').map(encodeURIComponent).join('/') + + for (const content of revisions.slice(1)) { + // recent versions are thinned out to one per two seconds + await new Promise((resolve) => setTimeout(resolve, 3500)) + const written = await request.put(`/remote.php/dav/files/${apiUser()}${path}`, { + headers: apiHeaders(), + data: content, + }) + expect(written.ok(), 'writing a revision').toBeTruthy() + } + + return note.id } /** diff --git a/src/components/NotePlain.vue b/src/components/NotePlain.vue index 16d298696..236d001f6 100644 --- a/src/components/NotePlain.vue +++ b/src/components/NotePlain.vue @@ -221,6 +221,7 @@ export default { document.addEventListener('visibilitychange', this.onVisibilityChange) subscribe('files_versions:restore:requested', this.onFileRestoreRequested) subscribe('files_versions:restore:restored', this.onFileRestored) + subscribe('files_versions:restore:failed', this.onFileRestoreFailed) }, unmounted() { @@ -233,6 +234,7 @@ export default { this.onUpdateTitle(null) unsubscribe('files_versions:restore:requested', this.onFileRestoreRequested) unsubscribe('files_versions:restore:restored', this.onFileRestored) + unsubscribe('files_versions:restore:failed', this.onFileRestoreFailed) }, methods: { @@ -339,22 +341,23 @@ export default { }, interval * 1000) }, - refreshNote() { - if (!this.note) { - this.startRefreshTimer() - return - } - if (this.note.unsaved && !this.note.conflict) { - this.startRefreshTimer() - return - } - refreshNote(parseInt(this.noteId), this.etag).then((etag) => { + async refreshNote() { + try { + if (!this.note) { + return + } + if (this.note.unsaved && !this.note.conflict) { + return + } + + const etag = await refreshNote(parseInt(this.noteId), this.etag) if (etag) { this.etag = etag this.$forceUpdate() } + } finally { this.startRefreshTimer() - }) + } }, onEdit(newContent) { @@ -432,24 +435,38 @@ export default { this.showConflict = false }, - async onFileRestoreRequested(event) { - const { fileInfo } = event + // the node of a restore carries a numeric fileid, a version a string fileId + isCurrentNote(fileId) { + return this.note && Number(fileId) === this.note.id + }, - if (!this.note || fileInfo.id !== this.note.id) { + onFileRestoreRequested({ node }) { + if (!this.isCurrentNote(node?.fileid)) { return } this.loading = true }, - async onFileRestored(version) { - if (!this.note || version.fileId !== this.note.id) { + onFileRestoreFailed(version) { + if (!this.isCurrentNote(version?.fileId)) { return } - this.refreshNote() this.loading = false }, + + async onFileRestored({ node }) { + if (!this.isCurrentNote(node?.fileid)) { + return + } + + try { + await this.refreshNote() + } finally { + this.loading = false + } + }, }, } diff --git a/src/components/NoteRich.vue b/src/components/NoteRich.vue index 4afd281ae..724bbd109 100644 --- a/src/components/NoteRich.vue +++ b/src/components/NoteRich.vue @@ -66,6 +66,7 @@ export default { subscribe('files:node:updated', this.fileUpdated) subscribe('files_versions:restore:requested', this.onFileRestoreRequested) subscribe('files_versions:restore:restored', this.onFileRestored) + subscribe('files_versions:restore:failed', this.onFileRestoreFailed) }, unmounted() { @@ -73,6 +74,7 @@ export default { unsubscribe('files:node:updated', this.fileUpdated) unsubscribe('files_versions:restore:requested', this.onFileRestoreRequested) unsubscribe('files_versions:restore:restored', this.onFileRestored) + unsubscribe('files_versions:restore:failed', this.onFileRestoreFailed) }, methods: { @@ -158,36 +160,41 @@ export default { return title.length > 0 ? title : t('notes', 'New note') }, - async onFileRestoreRequested(event) { - const { fileInfo } = event + // the node of a restore carries a numeric fileid, a version a string fileId + isCurrentNote(fileId) { + return this.note && Number(fileId) === this.note.id + }, - if (!this.note || fileInfo.id !== this.note.id) { + onFileRestoreRequested({ node }) { + if (!this.isCurrentNote(node?.fileid)) { return } this.loading = true }, - async onFileRestored(version) { - if (!this.note || version.fileId !== this.note.id) { + onFileRestoreFailed(version) { + if (!this.isCurrentNote(version?.fileId)) { return } - const etag = await refreshNote(parseInt(this.noteId), this.etag) + this.loading = false + }, - if (etag) { - this.etag = etag + async onFileRestored({ node }) { + if (!this.isCurrentNote(node?.fileid)) { + return } - const autoResolve = setInterval(() => { - const el = document.querySelector('[data-cy="resolveServerVersion"]') + try { + const etag = await refreshNote(parseInt(this.noteId), this.etag) - if (el) { - el.click() - clearInterval(autoResolve) + if (etag) { + this.etag = etag } - }, 200) - this.loading = false + } finally { + this.loading = false + } }, }, } From 54802103d00a327624887d0a470b83d8a79c297a Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Mon, 17 Aug 2026 22:51:33 +0200 Subject: [PATCH 8/8] test(notes): cover the sidebar tab helpers Cover the tab allow-list and ordering, and the element bootstrap with its deduplication, failure and timeout paths. Assisted-by: Claude Code:claude-opus-5[1m] Signed-off-by: Andy Scherzinger --- src/tests/sidebarTabs.spec.js | 195 ++++++++++++++++++++++++++++++++++ 1 file changed, 195 insertions(+) create mode 100644 src/tests/sidebarTabs.spec.js diff --git a/src/tests/sidebarTabs.spec.js b/src/tests/sidebarTabs.spec.js new file mode 100644 index 000000000..3b9641312 --- /dev/null +++ b/src/tests/sidebarTabs.spec.js @@ -0,0 +1,195 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { initializeSidebarTab, NOTE_SIDEBAR_TAB_IDS, selectNoteSidebarTabs, TAB_DEFINITION_TIMEOUT } from '../sidebarTabs.js' + +const ids = (tabs, context) => selectNoteSidebarTabs(tabs, context).map((tab) => tab.id) + +const node = { basename: 'A note.md' } + +describe('selectNoteSidebarTabs', () => { + it('keeps the tabs a note sidebar hosts', () => { + expect(NOTE_SIDEBAR_TAB_IDS).toEqual(['sharing', 'files_versions']) + }) + + it('drops every tab that is not on the allow-list', () => { + const tabs = [ + { id: 'sharing' }, + { id: 'activity' }, + { id: 'files_versions' }, + { id: 'comments' }, + ] + + expect(ids(tabs)).toEqual(['sharing', 'files_versions']) + }) + + it.each([ + ['nothing registered', []], + ['a registry that is not there yet', null], + ['entries without an id', [{}, null, undefined]], + ])('returns no tabs for %s', (_label, tabs) => { + expect(ids(tabs)).toEqual([]) + }) + + it('sorts by the order the registering apps asked for', () => { + const tabs = [ + { id: 'files_versions', order: 5 }, + { id: 'sharing', order: 1 }, + ] + + expect(ids(tabs)).toEqual(['sharing', 'files_versions']) + }) + + it('treats a missing order as zero', () => { + const tabs = [ + { id: 'files_versions', order: 1 }, + { id: 'sharing' }, + ] + + expect(ids(tabs)).toEqual(['sharing', 'files_versions']) + }) + + it('keeps a tab while the node it would judge is still loading', () => { + const tabs = [{ id: 'files_versions', enabled: () => false }] + + expect(ids(tabs, { node: null })).toEqual(['files_versions']) + }) + + it('asks the tab once the node is there', () => { + const enabled = vi.fn(() => true) + const folder = { basename: 'Notes' } + const view = { id: 'notes' } + + expect(ids([{ id: 'sharing', enabled }], { node, folder, view })).toEqual(['sharing']) + expect(enabled).toHaveBeenCalledWith({ node, folder, view }) + }) + + it('drops a tab that says it does not apply to the node', () => { + const tabs = [ + { id: 'sharing' }, + { id: 'files_versions', enabled: () => false }, + ] + + expect(ids(tabs, { node })).toEqual(['sharing']) + }) + + it('drops only the tab whose predicate throws', () => { + const tabs = [ + { id: 'sharing' }, + { id: 'files_versions', enabled: () => { throw new Error('no node for you') } }, + ] + + expect(ids(tabs, { node })).toEqual(['sharing']) + }) + + it('leaves the registry it was given alone', () => { + const tabs = [ + { id: 'files_versions', order: 5 }, + { id: 'sharing', order: 1 }, + ] + + selectNoteSidebarTabs(tabs, { node }) + + expect(tabs.map((tab) => tab.id)).toEqual(['files_versions', 'sharing']) + }) +}) + +describe('initializeSidebarTab', () => { + let elements + + /** + * @param {string} tagName the custom element a tab brings + * @return {object} the registry entry standing in for that element + */ + function element(tagName) { + if (!elements.has(tagName)) { + let resolve + const defined = new Promise((settle) => { + resolve = settle + }) + elements.set(tagName, { defined, resolve, isDefined: false }) + } + return elements.get(tagName) + } + + /** + * @param {string} tagName the custom element to report as defined + */ + function define(tagName) { + const entry = element(tagName) + entry.isDefined = true + entry.resolve() + } + + beforeEach(() => { + elements = new Map() + vi.spyOn(window.customElements, 'get') + .mockImplementation((tagName) => (element(tagName).isDefined ? class {} : undefined)) + vi.spyOn(window.customElements, 'whenDefined') + .mockImplementation((tagName) => element(tagName).defined) + }) + + afterEach(() => { + vi.restoreAllMocks() + vi.useRealTimers() + }) + + it('takes an element that is already in the registry as it is', async () => { + const onInit = vi.fn() + define('already-defined-tab') + + await expect(initializeSidebarTab({ id: 'sharing', tagName: 'already-defined-tab', onInit })).resolves.toBe(true) + expect(onInit).not.toHaveBeenCalled() + }) + + it('waits for the element the tab promises to define', async () => { + const onInit = vi.fn(async () => define('defining-tab')) + + await expect(initializeSidebarTab({ id: 'sharing', tagName: 'defining-tab', onInit })).resolves.toBe(true) + expect(onInit).toHaveBeenCalledTimes(1) + }) + + it('initializes an element once while opens overlap', async () => { + const onInit = vi.fn(async () => define('shared-tab')) + const tab = { id: 'sharing', tagName: 'shared-tab', onInit } + + const [first, second] = await Promise.all([initializeSidebarTab(tab), initializeSidebarTab(tab)]) + + expect([first, second]).toEqual([true, true]) + expect(onInit).toHaveBeenCalledTimes(1) + }) + + it('gives up on a tab whose initialization throws', async () => { + const onInit = vi.fn(async () => { + throw new Error('no element for you') + }) + + await expect(initializeSidebarTab({ id: 'sharing', tagName: 'throwing-tab', onInit })).resolves.toBe(false) + }) + + it('gives up on a tab that never defines its element', async () => { + vi.useFakeTimers() + const tab = { id: 'sharing', tagName: 'silent-tab', onInit: vi.fn() } + + const usable = initializeSidebarTab(tab) + await vi.advanceTimersByTimeAsync(TAB_DEFINITION_TIMEOUT + 1) + + await expect(usable).resolves.toBe(false) + }) + + it('lets a tab that failed try again on the next open', async () => { + const onInit = vi.fn() + .mockImplementationOnce(async () => { + throw new Error('not this time') + }) + .mockImplementationOnce(async () => define('retried-tab')) + const tab = { id: 'sharing', tagName: 'retried-tab', onInit } + + await expect(initializeSidebarTab(tab)).resolves.toBe(false) + await expect(initializeSidebarTab(tab)).resolves.toBe(true) + expect(onInit).toHaveBeenCalledTimes(2) + }) +})