mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-09-20 18:51:26 -03:00
refactor(sidebar): put the update check first, group the folder entries (#999)
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.
This commit is contained in:
@@ -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';
|
||||
// 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';
|
||||
}
|
||||
|
||||
// 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';
|
||||
}
|
||||
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) {
|
||||
|
||||
@@ -205,16 +205,21 @@
|
||||
</div>
|
||||
|
||||
<!-- Sidebar Folder Context Menu -->
|
||||
<!-- Order: the content action (update check) first, then the folder operations
|
||||
as one group, then the destructive action behind its own divider. The
|
||||
dividers are collapsed by SidebarManager when a group is hidden on the
|
||||
current page (recipes keep only the update check). -->
|
||||
<div id="sidebarFolderContextMenu" class="context-menu">
|
||||
<div class="context-menu-item" data-action="check-folder-updates">
|
||||
<i class="fas fa-bell"></i> <span>{{ t('sidebar.folderUpdateCheck.label') }}</span>
|
||||
</div>
|
||||
<div class="context-menu-separator"></div>
|
||||
<div class="context-menu-item" data-action="create-subfolder">
|
||||
<i class="fas fa-folder-plus"></i> <span>{{ t('sidebar.newSubfolder') }}</span>
|
||||
</div>
|
||||
<div class="context-menu-item" data-action="rename-folder">
|
||||
<i class="fas fa-i-cursor"></i> <span>{{ t('sidebar.renameFolder') }}</span>
|
||||
</div>
|
||||
<div class="context-menu-item" data-action="check-folder-updates">
|
||||
<i class="fas fa-bell"></i> <span>{{ t('sidebar.folderUpdateCheck.label') }}</span>
|
||||
</div>
|
||||
<div class="context-menu-separator"></div>
|
||||
<div class="context-menu-item delete-item" data-action="delete-folder">
|
||||
<i class="fas fa-trash"></i> <span>{{ t('sidebar.deleteFolder') }}</span>
|
||||
|
||||
@@ -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 = `
|
||||
<div id="sidebarFolderContextMenu" class="context-menu">
|
||||
<div class="context-menu-item" data-action="check-folder-updates"></div>
|
||||
<div class="context-menu-separator"></div>
|
||||
<div class="context-menu-item" data-action="create-subfolder"></div>
|
||||
<div class="context-menu-item" data-action="rename-folder"></div>
|
||||
<div class="context-menu-separator"></div>
|
||||
<div class="context-menu-item delete-item" data-action="delete-folder"></div>
|
||||
</div>`;
|
||||
|
||||
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 = `
|
||||
<div id="sidebarFolderContextMenu" class="context-menu">
|
||||
<div class="context-menu-separator"></div>
|
||||
<div class="context-menu-item" data-action="a"></div>
|
||||
<div class="context-menu-separator"></div>
|
||||
<div class="context-menu-separator"></div>
|
||||
<div class="context-menu-item" data-action="b"></div>
|
||||
<div class="context-menu-separator"></div>
|
||||
</div>`;
|
||||
const manager = createManager(createApiClient());
|
||||
|
||||
manager._updateContextMenuSeparators(document.getElementById('sidebarFolderContextMenu'));
|
||||
|
||||
expect(separators().map(visible)).toEqual([false, true, false, false]);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -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('<!-- Sidebar View Options Menu -->')
|
||||
);
|
||||
|
||||
const sequence = [...menuHtml.matchAll(/<div class="([^"]+)"([^>]*)>/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(/<div class="([^"]*)"\s+data-action="delete-folder"/);
|
||||
expect(deleteEntry).not.toBeNull();
|
||||
expect(deleteEntry[1]).toContain('delete-item');
|
||||
expect(sequence[sequence.length - 1]).toBe('delete-folder');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user