From cc25bb3dc25ea86c43ca1638ebdb81d2110ecc11 Mon Sep 17 00:00:00 2001 From: Will Miao Date: Tue, 15 Sep 2026 20:25:08 +0800 Subject: [PATCH] refactor(sidebar): put the update check first, group the folder entries (#999) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The folder context menu now reads: the content action (check for updates) on top, then the folder operations as one group (new subfolder, rename), then the destructive entry behind its own divider. Gating the three folder entries per page could leave the menu with dangling separators — the recipes sidebar hides all of them and keeps only the update check, which already rendered one stray divider before this change and would have rendered two after it. Add _updateContextMenuSeparators: a divider survives only when a visible entry sits on both sides, and a run of consecutive ones collapses to a single line. The three per-item display toggles fold into one loop. The template order is guarded by a regression test that parses templates/components/context_menu.html, plus behaviour tests for the divider collapsing. --- static/js/components/SidebarManager.js | 62 +++++++++++----- templates/components/context_menu.html | 11 ++- .../sidebarManager.folderManagement.test.js | 72 +++++++++++++++++++ .../sidebarFolderContextMenu.test.js | 44 ++++++++++++ 4 files changed, 169 insertions(+), 20 deletions(-) create mode 100644 tests/frontend/regression/sidebarFolderContextMenu.test.js diff --git a/static/js/components/SidebarManager.js b/static/js/components/SidebarManager.js index 10214b35..5e2e94c5 100644 --- a/static/js/components/SidebarManager.js +++ b/static/js/components/SidebarManager.js @@ -1635,24 +1635,18 @@ export class SidebarManager { const menu = document.getElementById('sidebarFolderContextMenu'); if (!menu) return; - // Folder creation is only available on pages backed by model library - // roots (not recipes, which have no on-disk folder management). - const createItem = menu.querySelector('[data-action="create-subfolder"]'); - if (createItem) { - createItem.style.display = this._supportsFolderManagement() ? '' : 'none'; - } - - // Deletion is gated the same way: recipes have virtual folders only. - const deleteItem = menu.querySelector('[data-action="delete-folder"]'); - if (deleteItem) { - deleteItem.style.display = this._supportsFolderManagement() ? '' : 'none'; - } - - // Renaming an on-disk folder is likewise library-only. - const renameItem = menu.querySelector('[data-action="rename-folder"]'); - if (renameItem) { - renameItem.style.display = this._supportsFolderManagement() ? '' : 'none'; + // The folder operations are only available on pages backed by model + // library roots (recipes have virtual folders only). Their dividers are + // collapsed afterwards so such a page shows the update check alone + // instead of dangling separators. + const supportsFolderManagement = this._supportsFolderManagement(); + for (const action of ['create-subfolder', 'rename-folder', 'delete-folder']) { + const item = menu.querySelector(`[data-action="${action}"]`); + if (item) { + item.style.display = supportsFolderManagement ? '' : 'none'; + } } + this._updateContextMenuSeparators(menu); menu.style.left = `${x}px`; menu.style.top = `${y}px`; @@ -1672,6 +1666,40 @@ export class SidebarManager { }, 0); } + /** + * Hide separators that no longer divide anything. + * + * Context-menu entries are gated per page, so a divider can end up + * leading, trailing or doubled once its group is hidden — the recipes page, + * for example, keeps only "check for updates". A separator survives only + * when a visible entry sits on both of its sides, and a run of consecutive + * separators collapses to a single line. + */ + _updateContextMenuSeparators(menu) { + const children = [...menu.children]; + const isSeparator = (element) => element.classList.contains('context-menu-separator'); + const visibleIndexes = children + .map((element, index) => (!isSeparator(element) && element.style.display !== 'none' ? index : -1)) + .filter((index) => index !== -1); + + const first = visibleIndexes[0]; + const last = visibleIndexes[visibleIndexes.length - 1]; + let inSeparatorRun = false; + + children.forEach((element, index) => { + if (!isSeparator(element)) { + inSeparatorRun = false; + return; + } + const keep = visibleIndexes.length >= 2 + && index > first + && index < last + && !inSeparatorRun; + element.style.display = keep ? '' : 'none'; + inSeparatorRun = true; + }); + } + _closeFolderContextMenu() { const menu = document.getElementById('sidebarFolderContextMenu'); if (menu) { diff --git a/templates/components/context_menu.html b/templates/components/context_menu.html index 9e5bd2f1..86f8188e 100644 --- a/templates/components/context_menu.html +++ b/templates/components/context_menu.html @@ -205,16 +205,21 @@ +
+
+ {{ t('sidebar.folderUpdateCheck.label') }} +
+
{{ t('sidebar.newSubfolder') }}
{{ t('sidebar.renameFolder') }}
-
- {{ t('sidebar.folderUpdateCheck.label') }} -
{{ t('sidebar.deleteFolder') }} diff --git a/tests/frontend/components/sidebarManager.folderManagement.test.js b/tests/frontend/components/sidebarManager.folderManagement.test.js index bfc5d636..00e6c742 100644 --- a/tests/frontend/components/sidebarManager.folderManagement.test.js +++ b/tests/frontend/components/sidebarManager.folderManagement.test.js @@ -846,3 +846,75 @@ describe('SidebarManager folder rename', () => { }); }); +describe('SidebarManager folder context menu layout', () => { + // Mirrors templates/components/context_menu.html: the update check on top, + // the folder operations as one group, delete last behind its own divider. + const MENU_HTML = ` +
+
+
+
+
+
+
+
`; + + const separators = () => [...document.querySelectorAll('#sidebarFolderContextMenu .context-menu-separator')]; + const item = (action) => document.querySelector(`#sidebarFolderContextMenu [data-action="${action}"]`); + const visible = (el) => el.style.display !== 'none'; + + beforeEach(() => { + localStorage.clear(); + document.body.innerHTML = MENU_HTML; + state.global.settings = {}; + vi.clearAllMocks(); + }); + + it('keeps both dividers on a library page', () => { + const manager = createManager(createApiClient()); + + manager._showFolderContextMenu(10, 10, 'empty'); + + expect(separators().map(visible)).toEqual([true, true]); + expect(visible(item('create-subfolder'))).toBe(true); + expect(visible(item('rename-folder'))).toBe(true); + expect(visible(item('delete-folder'))).toBe(true); + + manager._closeFolderContextMenu(); + }); + + it('collapses both dividers when the page has no folder management', () => { + const apiClient = createApiClient(); + apiClient.apiConfig.config.supportsFolderManagement = false; + const manager = createManager(apiClient); + + manager._showFolderContextMenu(10, 10, 'empty'); + + expect(visible(item('check-folder-updates'))).toBe(true); + expect(visible(item('create-subfolder'))).toBe(false); + expect(visible(item('rename-folder'))).toBe(false); + expect(visible(item('delete-folder'))).toBe(false); + // Nothing left to divide: the update check stands alone + expect(separators().map(visible)).toEqual([false, false]); + + manager._closeFolderContextMenu(); + }); + + it('drops leading, trailing and doubled separators', () => { + document.body.innerHTML = ` +
+
+
+
+
+
+
+
`; + const manager = createManager(createApiClient()); + + manager._updateContextMenuSeparators(document.getElementById('sidebarFolderContextMenu')); + + expect(separators().map(visible)).toEqual([false, true, false, false]); + }); +}); + diff --git a/tests/frontend/regression/sidebarFolderContextMenu.test.js b/tests/frontend/regression/sidebarFolderContextMenu.test.js new file mode 100644 index 00000000..cecba33a --- /dev/null +++ b/tests/frontend/regression/sidebarFolderContextMenu.test.js @@ -0,0 +1,44 @@ +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'fs'; +import path from 'path'; + +// Regression guard for the sidebar folder context-menu layout: the update check +// sits on top, the folder operations form a single group, and the destructive +// entry stays last behind its own divider. SidebarManager gates those groups +// per page and collapses the dividers when a group is hidden, so a reorder here +// also changes what the recipes page shows. +describe('Sidebar folder context menu layout', () => { + const repoRoot = path.resolve(__dirname, '../../..'); + const html = readFileSync( + path.join(repoRoot, 'templates/components/context_menu.html'), + 'utf-8' + ); + + const menuHtml = html.slice( + html.indexOf('id="sidebarFolderContextMenu"'), + html.indexOf('') + ); + + const sequence = [...menuHtml.matchAll(/
]*)>/g)].map(([, classes, rest]) => { + if (classes.includes('context-menu-separator')) return 'separator'; + return /data-action="([^"]+)"/.exec(rest)?.[1] || null; + }); + + it('keeps the update check first and the folder operations grouped', () => { + expect(sequence).toEqual([ + 'check-folder-updates', + 'separator', + 'create-subfolder', + 'rename-folder', + 'separator', + 'delete-folder', + ]); + }); + + it('keeps the destructive entry last and visually marked', () => { + const deleteEntry = menuHtml.match(/