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..cb899035f --- /dev/null +++ b/playwright/e2e/note-sidebar.spec.ts @@ -0,0 +1,128 @@ +/** + * 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, setNoteMode, 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-share-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]') +} + +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() + 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('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)) + + 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 }) + }) + + // The editor's own actions menu only exists in the markdown editor; the rich + // editor brings its own menu bar. + test.describe('markdown editor', () => { + test.beforeEach(async ({ page, request }) => { + await setNoteMode(request, 'edit') + await page.reload() + }) + + test.afterEach(async ({ request }) => { + await setNoteMode(request, 'rich') + }) + + test('opens the sidebar from the editor actions menu', async ({ page }, testInfo: TestInfo) => { + await createNote(page, uniqueTitle('sidebar-editor-menu', testInfo)) + + await page.locator('.action-buttons .action-item__menutoggle').first().click() + await page.getByRole('menuitem', { name: 'Open sidebar', exact: true }).click() + + 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/support/note.ts b/playwright/support/note.ts index 83eb612a6..f72c39c2a 100644 --- a/playwright/support/note.ts +++ b/playwright/support/note.ts @@ -3,7 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import type { Locator, Page, TestInfo } from '@playwright/test' +import type { APIRequestContext, Locator, Page, TestInfo } from '@playwright/test' import { expect } from '@playwright/test' import { NoteEditor } from './sections/NoteEditor.ts' @@ -12,6 +12,29 @@ export function uniqueTitle(prefix: string, testInfo: TestInfo): string { return `Playwright ${prefix} ${testInfo.parallelIndex}-${Date.now()}` } +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')}` } +} + +/** + * Switch the editor the app renders: `rich`, `edit` or `preview`. + * + * Takes the isolated `request` fixture rather than `page.request`, whose basic + * auth would replace the session cookie the browser is logged in with. + * + * @param request The request fixture to use + * @param mode The editor mode to switch to + */ +export async function setNoteMode(request: APIRequestContext, mode: string): Promise { + const response = await request.put('/index.php/apps/notes/api/v1/settings', { + headers: apiHeaders(), + data: { noteMode: mode }, + }) + expect(response.ok(), `switching to the ${mode} editor`).toBeTruthy() +} + export function currentNoteId(page: Page): number | null { const match = page.url().match(/\/note\/(\d+)(?:\?.*)?$/) return match ? Number(match[1]) : null @@ -26,6 +49,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) @@ -43,9 +73,7 @@ export async function waitForNoteRoute(page: Page, previousNoteId: number | null * @param page The page object to use */ export async function deleteAllNotes(page: Page): Promise { - const user = process.env.NC_USER ?? 'admin' - const password = process.env.NC_PASS ?? 'admin' - const headers = { Authorization: `Basic ${Buffer.from(`${user}:${password}`).toString('base64')}` } + const headers = apiHeaders() const response = await page.request.get('/index.php/apps/notes/api/v1/notes', { headers }) expect(response.ok()).toBeTruthy() diff --git a/src/components/NoteItem.vue b/src/components/NoteItem.vue index 535bfd6e0..635b5af52 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 }, diff --git a/src/components/NoteShareSidebar.vue b/src/components/NoteShareSidebar.vue index 095ee0efc..dec29c11f 100644 --- a/src/components/NoteShareSidebar.vue +++ b/src/components/NoteShareSidebar.vue @@ -9,20 +9,24 @@ data-cy-notes-share-sidebar forceMenu :loading="isOpen && loading" - :name="note?.title || t('notes', 'Share')" + :name="note?.title || t('notes', 'Note')" noToggle :open="isOpen" @closed="onClosed" @update:open="onToggle" > - + + + + @@ -31,28 +35,28 @@ - + - {{ error || t('notes', 'Unable to load the selected note for sharing.') }} + {{ contextError || t('notes', 'Unable to load the selected note.') }} - + - {{ t('notes', 'Sharing is not available right now.') }} + {{ tabError || t('notes', 'Sharing and versions are not available right now.') }} @@ -65,8 +69,10 @@ import NcAppSidebarTab from '@nextcloud/vue/components/NcAppSidebarTab' import NcEmptyContent from '@nextcloud/vue/components/NcEmptyContent' import NcIconSvgWrapper from '@nextcloud/vue/components/NcIconSvgWrapper' import NcLoadingIcon from '@nextcloud/vue/components/NcLoadingIcon' -import ShareVariantOutlineIcon from 'vue-material-design-icons/ShareVariantOutline.vue' +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 store from '../store.js' import { fetchDavNode } from '../WebdavService.js' @@ -79,7 +85,8 @@ export default { NcEmptyContent, NcIconSvgWrapper, NcLoadingIcon, - ShareVariantOutlineIcon, + FileOutlineIcon, + NoteSidebarSubname, }, data() { @@ -89,8 +96,9 @@ export default { contextRequestToken: 0, currentFolder: null, currentNode: null, - initializingTabs: new Set(), + pendingTabs: new Map(), initializedTabs: new Set(), + failedTabs: new Set(), isOpen: false, loadingContext: false, loadingTab: false, @@ -100,10 +108,6 @@ export default { }, computed: { - error() { - return this.tabError || this.contextError - }, - loading() { return this.loadingContext || this.loadingTab }, @@ -115,8 +119,16 @@ export default { return store.notes.getNote(this.noteId) }, - sharingTab() { - return getSidebarTabs().find((tab) => tab.id === 'sharing') || null + availableTabs() { + return selectNoteSidebarTabs(getSidebarTabs(), { + node: this.currentNode, + folder: this.currentFolder, + view: this.currentView, + }) + }, + + tabs() { + return this.availableTabs.filter((tab) => !this.failedTabs.has(tab.tagName)) }, currentView() { @@ -127,58 +139,96 @@ 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) + }, + }, + mounted() { + // the share event is kept so anything already emitting it keeps working subscribe('notes:share:open', this.onShareOpen) + subscribe('notes:sidebar:open', this.onSidebarOpen) }, unmounted() { unsubscribe('notes:share:open', this.onShareOpen) + unsubscribe('notes:sidebar:open', this.onSidebarOpen) }, methods: { - async initializeSharingTab() { - const tab = this.sharingTab - if (!tab) { + async initializeTabs() { + const tabs = this.availableTabs + if (tabs.length === 0) { this.loadingTab = false - this.tabError = this.t('notes', 'Sharing is not available right now.') return } + // 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))) + + 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)) { - this.loadingTab = false - this.tabError = '' - return + return true } - if (this.initializingTabs.has(tab.tagName)) { - this.loadingTab = true - return + 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 } - this.initializingTabs.add(tab.tagName) - this.loadingTab = true - this.tabError = '' + const initialization = this.defineTabElement(tab) + this.pendingTabs.set(tab.tagName, initialization) + + try { + return await initialization + } finally { + this.pendingTabs.delete(tab.tagName) + } + }, + /** + * @param {object} tab a registered Files sidebar tab + * @return {Promise} whether its custom element got defined + */ + async defineTabElement(tab) { try { await tab.onInit?.() await window.customElements.whenDefined(tab.tagName) this.initializedTabs.add(tab.tagName) + return true } catch (error) { - logger.error('Failed to initialize the sharing sidebar tab in Notes', { error }) - this.tabError = this.t('notes', 'Failed to load the sharing sidebar.') - } finally { - this.initializingTabs.delete(tab.tagName) - this.loadingTab = false + logger.error('Failed to initialize a sidebar tab in Notes', { error, tab: tab.id }) + this.failedTabs.add(tab.tagName) + return false } }, - async loadShareContext() { + async loadNodeContext() { const internalPath = this.note?.internalPath if (!internalPath) { this.loadingContext = false this.currentNode = null this.currentFolder = null - this.contextError = this.t('notes', 'Unable to load the selected note for sharing.') + this.contextError = this.t('notes', 'Unable to load the selected note.') return } @@ -193,7 +243,7 @@ export default { try { folder = await fetchDavNode(node.dirname || '/') } catch (error) { - logger.error('Failed to load the parent folder for the Notes sharing sidebar', { error }) + logger.error('Failed to load the parent folder for the Notes sidebar', { error }) } if (requestToken !== this.contextRequestToken) { @@ -207,10 +257,10 @@ export default { return } - logger.error('Failed to load the selected note for the Notes sharing sidebar', { error }) + logger.error('Failed to load the selected note for the Notes sidebar', { error }) this.currentNode = null this.currentFolder = null - this.contextError = this.t('notes', 'Unable to load the selected note for sharing.') + this.contextError = this.t('notes', 'Unable to load the selected note.') } finally { if (requestToken === this.contextRequestToken) { this.loadingContext = false @@ -218,10 +268,29 @@ export default { } }, - async onShareOpen({ noteId }) { + /** + * 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.noteId = Number(noteId) - this.activeTab = 'sharing' this.isOpen = true this.contextError = '' this.tabError = '' @@ -229,15 +298,17 @@ export default { this.currentFolder = null this.loadingContext = false this.loadingTab = false + this.failedTabs.clear() + this.activeTab = this.resolveTab(tab, this.tabs) - if (!this.sharingTab) { - await this.initializeSharingTab() + if (this.availableTabs.length === 0) { + await this.initializeTabs() return } await Promise.all([ - this.initializeSharingTab(), - this.loadShareContext(), + this.initializeTabs(), + this.loadNodeContext(), ]) }, diff --git a/src/components/NoteSidebarSubname.vue b/src/components/NoteSidebarSubname.vue new file mode 100644 index 000000000..d907d3919 --- /dev/null +++ b/src/components/NoteSidebarSubname.vue @@ -0,0 +1,74 @@ + + + + + + + diff --git a/src/sidebarTabs.js b/src/sidebarTabs.js new file mode 100644 index 000000000..78c615ff6 --- /dev/null +++ b/src/sidebarTabs.js @@ -0,0 +1,51 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import logger from './Logger.js' + +/** + * Files sidebar tabs the Notes sidebar hosts, and nothing else. + * + * Notes dispatches OCA\Files\Event\LoadSidebar when rendering its page, so every + * app that registers a sidebar tab has registered one by the time this runs — + * including tabs that make no sense for a note. This is an allow-list so a newly + * installed app cannot start appearing in the Notes sidebar unannounced. + * + * @type {string[]} + */ +export const NOTE_SIDEBAR_TAB_IDS = ['sharing', 'files_versions'] + +/** + * The tabs to render, in the order the registering apps asked for. + * + * A tab's own `enabled()` predicate has the final say — the versions tab for + * instance hides itself on public shares and for anything that is not a file — + * but it needs a node to judge, so while the node is still loading the tabs are + * kept and filtered again once it arrives. A predicate that throws is treated as + * "not usable" rather than being allowed to take the sidebar down. + * + * @param {Array} tabs all registered tabs, from getSidebarTabs() + * @param {object} context what the tab is being asked about + * @param {object|null} context.node the note's DAV node, null while loading + * @param {object|null} context.folder the note's parent folder + * @param {object|null} context.view the pseudo view Notes reports + * @return {Array} tabs to render, sorted by their declared order + */ +export function selectNoteSidebarTabs(tabs, { node = null, folder = null, view = null } = {}) { + return (tabs ?? []) + .filter((tab) => NOTE_SIDEBAR_TAB_IDS.includes(tab?.id)) + .filter((tab) => { + if (typeof tab.enabled !== 'function' || node === null) { + return true + } + try { + return tab.enabled({ node, folder, view }) + } catch (error) { + logger.error('Sidebar tab predicate failed in Notes, dropping the tab', { error, tab: tab.id }) + return false + } + }) + .sort((a, b) => (a.order ?? 0) - (b.order ?? 0)) +}