From b940990d1b37782ca4a024d45aa9e6921fe7d6ae Mon Sep 17 00:00:00 2001 From: Frank Karlitschek Date: Thu, 6 Aug 2026 19:18:16 +0200 Subject: [PATCH] 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. * NoteShareSidebar 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. notes:share:open is kept as a thin wrapper so anything 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. The data-cy-notes-share-sidebar hook is deliberately unchanged, since playwright/e2e/basic.spec.ts asserts on it. Co-Authored-By: Claude Opus 5 (1M context) --- src/components/NoteItem.vue | 14 ++++ src/components/NoteShareSidebar.vue | 108 +++++++++++++++++----------- src/sidebarTabs.js | 48 +++++++++++++ 3 files changed, 129 insertions(+), 41 deletions(-) create mode 100644 src/sidebarTabs.js 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') }} + + @@ -65,8 +65,9 @@ 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 logger from '../Logger.js' +import { selectNoteSidebarTabs } from '../sidebarTabs.js' import store from '../store.js' import { fetchDavNode } from '../WebdavService.js' @@ -79,7 +80,7 @@ export default { NcEmptyContent, NcIconSvgWrapper, NcLoadingIcon, - ShareVariantOutlineIcon, + FileOutlineIcon, }, data() { @@ -115,8 +116,12 @@ export default { return store.notes.getNote(this.noteId) }, - sharingTab() { - return getSidebarTabs().find((tab) => tab.id === 'sharing') || null + tabs() { + return selectNoteSidebarTabs(getSidebarTabs(), { + node: this.currentNode, + folder: this.currentFolder, + view: this.currentView, + }) }, currentView() { @@ -128,57 +133,74 @@ export default { }, 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.tabs + if (tabs.length === 0) { this.loadingTab = false - this.tabError = this.t('notes', 'Sharing is not available right now.') + this.tabError = this.t('notes', 'Sharing and versions are 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)) { + // another open is already awaiting this one this.loadingTab = true - return + return true } this.initializingTabs.add(tab.tagName) this.loadingTab = true - this.tabError = '' 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.') + logger.error('Failed to initialize a sidebar tab in Notes', { error, tab: tab.id }) + return false } finally { this.initializingTabs.delete(tab.tagName) - this.loadingTab = 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 +215,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 +229,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 +240,14 @@ export default { } }, - async onShareOpen({ noteId }) { + onShareOpen({ noteId }) { + return this.onSidebarOpen({ noteId, tab: 'sharing' }) + }, + + async onSidebarOpen({ noteId, tab = 'sharing' }) { this.contextRequestToken += 1 this.noteId = Number(noteId) - this.activeTab = 'sharing' + this.activeTab = tab this.isOpen = true this.contextError = '' this.tabError = '' @@ -230,14 +256,14 @@ export default { this.loadingContext = false this.loadingTab = false - if (!this.sharingTab) { - await this.initializeSharingTab() + if (this.tabs.length === 0) { + await this.initializeTabs() return } await Promise.all([ - this.initializeSharingTab(), - this.loadShareContext(), + this.initializeTabs(), + this.loadNodeContext(), ]) }, diff --git a/src/sidebarTabs.js b/src/sidebarTabs.js new file mode 100644 index 000000000..016cc1844 --- /dev/null +++ b/src/sidebarTabs.js @@ -0,0 +1,48 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +/** + * 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 { + return false + } + }) + .sort((a, b) => (a.order ?? 0) - (b.order ?? 0)) +}