feat: manage view rename/delete, unified views submenu, stable checkbox list, dead code cleanup
Fix 1: Wire rename click handler in Manage View panel - openManageViewPanel() now adds click handler on #manage-view-name - Replaces h2 with inline manage-view-panel__name-input on click - On Enter: validates (non-empty, max 30, not reserved, not duplicate) then PATCHes /api/settings, updates _activeView, re-renders panel - On Escape / blur: reverts to text display - Add CSS for .manage-view-panel__delete-btn and .manage-view-panel__confirm-text Fix 2: Add delete button in Manage View panel header - Adds trash button (id=manage-view-delete-btn) next to view name - Clicking shows inline 'Delete this view? [Yes] [No]' confirmation - On confirm: PATCH /api/settings to remove view, close panel, switch to 'all' view, show toast confirming deletion Fix 3: Remove rename affordance from settings tab - Remove cursor:pointer from .views-settings-name - Remove .views-settings-name:hover hover/underline styles - Remove .views-settings-rename-input CSS class (dead code) Fix 4: Merge Add to View + Remove from View into unified Views submenu - Remove 'remove-from-view' item from FLYOUT_MENU_MAP 'user' entry - _openFlyoutSubmenu now shows ALL user views (no longer skips the current active view) — unified toggle-checkmark submenu - Submenu already stays open during toggling (existing behavior) - Update old test to match new correct behavior Fix 5: Fix checkbox re-render layout thrash in Manage View panel - renderManageViewList onChange .then() no longer calls renderManageViewList() - Instead updates summaryEl.textContent in-place with current counts - Prevents position jumps when toggling checkboxes in long lists Fix 6: Clean up dead CSS - Remove .add-sessions-tile + __icon + __label (old affordance tile) - Remove .manage-view-panel__close (HTML uses __close-btn) - Remove .manage-view-panel__title (HTML uses __name) - Update CSS comment 'Add Sessions Panel' -> 'Manage View Panel' - Keep .manage-view-panel__name-input (now used by Fix 1 rename input)
This commit is contained in:
Brian Krabach
+133
-14
@@ -143,7 +143,7 @@ let _flyoutRemoteId = null;
|
||||
* Data map of menu item definitions keyed by view type.
|
||||
* Each entry is an array of item config objects with:
|
||||
* { label, action, className?, separator? }
|
||||
* The 'user' view type gets the active view name injected at render time.
|
||||
* The 'user' view type uses a unified Views submenu (no separate Remove item).
|
||||
*/
|
||||
const FLYOUT_MENU_MAP = {
|
||||
'all': [
|
||||
@@ -154,7 +154,6 @@ const FLYOUT_MENU_MAP = {
|
||||
],
|
||||
'user': [
|
||||
{ label: 'Add to View\u2026', action: 'add-to-view', className: 'flyout-menu__item--has-submenu' },
|
||||
{ label: 'Remove from {viewName}', action: 'remove-from-view' },
|
||||
{ label: 'Hide', action: 'hide' },
|
||||
{ separator: true },
|
||||
{ label: 'Kill Session', action: 'kill', className: 'flyout-menu__item--danger' },
|
||||
@@ -2014,12 +2013,10 @@ function _openFlyoutSubmenu(triggerItem, unhideFirst) {
|
||||
var views = (_serverSettings && _serverSettings.views) || [];
|
||||
|
||||
var sessionKey = _flyoutSessionKey;
|
||||
// When in a user view, filter it out — the user already has "Remove from [ViewName]" for it
|
||||
var isInUserView = _activeView !== 'all' && _activeView !== 'hidden';
|
||||
// Show ALL user views with checkmarks — unified Views submenu
|
||||
var html = '';
|
||||
for (var i = 0; i < views.length; i++) {
|
||||
var v = views[i];
|
||||
if (isInUserView && v.name === _activeView) continue;
|
||||
var isIn = (v.sessions || []).indexOf(sessionKey) !== -1;
|
||||
html += '<button class="flyout-submenu__item" role="menuitem" data-view-index="' + i + '">';
|
||||
html += '<span class="flyout-submenu__check">' + (isIn ? '\u2713' : '') + '</span>';
|
||||
@@ -2352,6 +2349,7 @@ function _executeKill(name, remoteId) {
|
||||
/**
|
||||
* Open the Manage View panel for the active user view.
|
||||
* Only available for user views (not "All" or "Hidden").
|
||||
* Renders the view name (clickable to rename) and a delete button in the header.
|
||||
*/
|
||||
function openManageViewPanel() {
|
||||
if (_activeView === 'all' || _activeView === 'hidden') return;
|
||||
@@ -2359,23 +2357,133 @@ function openManageViewPanel() {
|
||||
var panel = $('manage-view-panel');
|
||||
if (!panel) return;
|
||||
|
||||
// Update title/name
|
||||
var nameEl = $('manage-view-name');
|
||||
if (nameEl) nameEl.textContent = _activeView;
|
||||
// Rebuild the name-row with view name + delete button
|
||||
var nameRow = panel.querySelector('.manage-view-panel__name-row');
|
||||
if (nameRow) {
|
||||
nameRow.innerHTML =
|
||||
'<h2 id="manage-view-name" class="manage-view-panel__name">' + escapeHtml(_activeView) + '</h2>' +
|
||||
'<button id="manage-view-delete-btn" class="manage-view-panel__delete-btn" ' +
|
||||
'title="Delete this view" aria-label="Delete view">\u2715</button>';
|
||||
}
|
||||
|
||||
renderManageViewList();
|
||||
panel.classList.remove('hidden');
|
||||
|
||||
// Close on backdrop click
|
||||
var backdrop = $('manage-view-backdrop');
|
||||
if (backdrop) {
|
||||
backdrop.onclick = closeManageViewPanel;
|
||||
}
|
||||
if (backdrop) backdrop.onclick = closeManageViewPanel;
|
||||
|
||||
// Close button at bottom
|
||||
var closeBtn = $('manage-view-close');
|
||||
if (closeBtn) {
|
||||
closeBtn.onclick = closeManageViewPanel;
|
||||
if (closeBtn) closeBtn.onclick = closeManageViewPanel;
|
||||
|
||||
// — Rename click handler on the view name —
|
||||
var nameEl = $('manage-view-name');
|
||||
if (nameEl) {
|
||||
nameEl.onclick = function() {
|
||||
var currentName = _activeView;
|
||||
// Replace h2 with an inline input for rename
|
||||
var input = document.createElement('input');
|
||||
input.type = 'text';
|
||||
input.className = 'manage-view-panel__name-input';
|
||||
input.value = currentName;
|
||||
input.maxLength = 30;
|
||||
input.setAttribute('aria-label', 'View name');
|
||||
if (nameEl.parentNode) nameEl.parentNode.replaceChild(input, nameEl);
|
||||
input.focus();
|
||||
input.select();
|
||||
|
||||
var _committed = false;
|
||||
|
||||
function commitRename() {
|
||||
if (_committed) return;
|
||||
var newName = input.value.trim();
|
||||
if (!newName || newName === currentName) { revertRename(); return; }
|
||||
if (newName.toLowerCase() === 'all' || newName.toLowerCase() === 'hidden') {
|
||||
showToast('Cannot use reserved name \'' + newName + '\'');
|
||||
input.focus(); return;
|
||||
}
|
||||
var views = (_serverSettings && _serverSettings.views) || [];
|
||||
if (views.find(function(v) { return v.name === newName; })) {
|
||||
showToast('View \'' + newName + '\' already exists');
|
||||
input.focus(); return;
|
||||
}
|
||||
_committed = true;
|
||||
var updatedViews = views.map(function(v) {
|
||||
return v.name === currentName ? { name: newName, sessions: v.sessions || [] } : v;
|
||||
});
|
||||
api('PATCH', '/api/settings', { views: updatedViews })
|
||||
.then(function() {
|
||||
if (_serverSettings) _serverSettings.views = updatedViews;
|
||||
_activeView = newName;
|
||||
api('PATCH', '/api/state', { active_view: newName }).catch(function() {});
|
||||
renderViewDropdown();
|
||||
openManageViewPanel();
|
||||
})
|
||||
.catch(function() {
|
||||
showToast('Failed to rename view');
|
||||
_committed = false;
|
||||
revertRename();
|
||||
});
|
||||
}
|
||||
|
||||
function revertRename() {
|
||||
if (_committed) return;
|
||||
openManageViewPanel();
|
||||
}
|
||||
|
||||
input.addEventListener('keydown', function(e) {
|
||||
if (e.key === 'Enter') { e.preventDefault(); commitRename(); }
|
||||
else if (e.key === 'Escape') { revertRename(); }
|
||||
});
|
||||
input.addEventListener('blur', function() {
|
||||
setTimeout(function() {
|
||||
if (document.activeElement !== input) {
|
||||
var newName = input.value.trim();
|
||||
if (newName && newName !== currentName) commitRename();
|
||||
else revertRename();
|
||||
}
|
||||
}, 150);
|
||||
});
|
||||
};
|
||||
}
|
||||
|
||||
// — Delete button handler —
|
||||
var deleteBtn = $('manage-view-delete-btn');
|
||||
if (deleteBtn) {
|
||||
deleteBtn.onclick = function(e) {
|
||||
e.stopPropagation();
|
||||
// Show inline confirmation in the name-row
|
||||
var nameRow2 = panel.querySelector('.manage-view-panel__name-row');
|
||||
if (!nameRow2) return;
|
||||
nameRow2.innerHTML =
|
||||
'<span class="manage-view-panel__confirm-text">Delete this view?</span>' +
|
||||
'<button id="manage-view-confirm-yes" class="manage-view-panel__close-btn" ' +
|
||||
'style="margin-left:8px">Yes</button>' +
|
||||
'<button id="manage-view-confirm-no" class="manage-view-panel__close-btn" ' +
|
||||
'style="margin-left:4px">No</button>';
|
||||
var yesBtn = $('manage-view-confirm-yes');
|
||||
var noBtn = $('manage-view-confirm-no');
|
||||
if (yesBtn) {
|
||||
yesBtn.onclick = function() {
|
||||
var viewToDelete = _activeView;
|
||||
var views = (_serverSettings && _serverSettings.views) || [];
|
||||
var updatedViews = views.filter(function(v) { return v.name !== viewToDelete; });
|
||||
api('PATCH', '/api/settings', { views: updatedViews })
|
||||
.then(function() {
|
||||
if (_serverSettings) _serverSettings.views = updatedViews;
|
||||
closeManageViewPanel();
|
||||
switchView('all');
|
||||
showToast('View \'' + viewToDelete + '\' deleted');
|
||||
renderViewDropdown();
|
||||
})
|
||||
.catch(function() { showToast('Failed to delete view'); });
|
||||
};
|
||||
}
|
||||
if (noBtn) {
|
||||
noBtn.onclick = function() { openManageViewPanel(); };
|
||||
}
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2511,7 +2619,18 @@ function renderManageViewList() {
|
||||
_serverSettings.views = updatedViews;
|
||||
if (patch.hidden_sessions) _serverSettings.hidden_sessions = patch.hidden_sessions;
|
||||
}
|
||||
renderManageViewList();
|
||||
// Update summary count in-place — do NOT re-render the full list (avoids layout thrash)
|
||||
var summaryEl = $('manage-view-summary');
|
||||
if (summaryEl) {
|
||||
var latestViews = (_serverSettings && _serverSettings.views) || [];
|
||||
var latestViewObj = null;
|
||||
for (var si = 0; si < latestViews.length; si++) {
|
||||
if (latestViews[si].name === _activeView) { latestViewObj = latestViews[si]; break; }
|
||||
}
|
||||
var latestViewSessions = (latestViewObj && latestViewObj.sessions) || [];
|
||||
var latestAllSessions = (_currentSessions || []).filter(function(s) { return !s.status; });
|
||||
summaryEl.textContent = latestAllSessions.length + ' sessions \u00b7 ' + latestViewSessions.length + ' in this view';
|
||||
}
|
||||
renderGrid(_currentSessions || []);
|
||||
})
|
||||
.catch(function(err) {
|
||||
|
||||
+29
-73
@@ -1823,7 +1823,7 @@ body {
|
||||
overflows from bottom:0 upward, and tile-body clips excess at the top — showing
|
||||
the bottom-most content (the prompt area) just like a real terminal. */
|
||||
|
||||
/* —— Add Sessions Panel —————————————————————————————————————————————————————————————————————————— */
|
||||
/* —— Manage View Panel —————————————————————————————————————————————————————————————————————————— */
|
||||
|
||||
.manage-view-panel {
|
||||
position: fixed;
|
||||
@@ -1861,31 +1861,7 @@ body {
|
||||
border-bottom: 1px solid var(--border-subtle);
|
||||
}
|
||||
|
||||
.manage-view-panel__title {
|
||||
font-size: 15px;
|
||||
font-weight: 600;
|
||||
color: var(--text);
|
||||
margin: 0;
|
||||
}
|
||||
|
||||
.manage-view-panel__close {
|
||||
width: 28px;
|
||||
height: 28px;
|
||||
display: flex;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
background: transparent;
|
||||
border: none;
|
||||
color: var(--text-muted);
|
||||
font-size: 18px;
|
||||
cursor: pointer;
|
||||
border-radius: 4px;
|
||||
}
|
||||
|
||||
.manage-view-panel__close:hover {
|
||||
background: var(--bg-surface);
|
||||
color: var(--text);
|
||||
}
|
||||
|
||||
.manage-view-panel__list {
|
||||
overflow-y: auto;
|
||||
@@ -2025,6 +2001,34 @@ body {
|
||||
max-width: 100%;
|
||||
}
|
||||
|
||||
.manage-view-panel__delete-btn {
|
||||
width: 24px;
|
||||
height: 24px;
|
||||
display: flex;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
background: transparent;
|
||||
border: 1px solid var(--border);
|
||||
border-radius: 4px;
|
||||
color: var(--text-muted);
|
||||
font-size: 14px;
|
||||
cursor: pointer;
|
||||
margin-left: 6px;
|
||||
flex-shrink: 0;
|
||||
}
|
||||
|
||||
.manage-view-panel__delete-btn:hover {
|
||||
border-color: var(--err);
|
||||
color: var(--err);
|
||||
background: rgba(248, 81, 73, 0.1);
|
||||
}
|
||||
|
||||
.manage-view-panel__confirm-text {
|
||||
font-size: 13px;
|
||||
color: var(--text-muted);
|
||||
margin-right: 4px;
|
||||
}
|
||||
|
||||
/* ============================================================
|
||||
Responsive overlay sidebar at <960px
|
||||
============================================================ */
|
||||
@@ -2208,17 +2212,11 @@ body {
|
||||
flex: 1;
|
||||
font-size: 13px;
|
||||
color: var(--text);
|
||||
cursor: pointer;
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
white-space: nowrap;
|
||||
}
|
||||
|
||||
.views-settings-name:hover {
|
||||
color: var(--accent);
|
||||
text-decoration: underline;
|
||||
}
|
||||
|
||||
.views-settings-count {
|
||||
font-size: 11px;
|
||||
color: var(--text-muted);
|
||||
@@ -2289,16 +2287,6 @@ body {
|
||||
color: var(--text-muted);
|
||||
}
|
||||
|
||||
.views-settings-rename-input {
|
||||
flex: 1;
|
||||
background: var(--bg);
|
||||
border: 1px solid var(--accent);
|
||||
border-radius: 4px;
|
||||
color: var(--text);
|
||||
font-size: 13px;
|
||||
padding: 3px 6px;
|
||||
outline: none;
|
||||
}
|
||||
|
||||
/* —— Tile Flyout Menu ————————————————————————————————————————————————————— */
|
||||
|
||||
@@ -2524,35 +2512,3 @@ body {
|
||||
margin: 4px 0;
|
||||
background: var(--border-subtle);
|
||||
}
|
||||
|
||||
/* —— Add Sessions affordance tile ————————————————————————————————————————— */
|
||||
|
||||
.add-sessions-tile {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
gap: 8px;
|
||||
min-height: 120px;
|
||||
background: transparent;
|
||||
border: 2px dashed var(--border-subtle);
|
||||
border-radius: 8px;
|
||||
color: var(--text-dim);
|
||||
font-size: 13px;
|
||||
cursor: pointer;
|
||||
transition: border-color var(--t-fast), color var(--t-fast);
|
||||
}
|
||||
|
||||
.add-sessions-tile:hover {
|
||||
border-color: var(--accent);
|
||||
color: var(--accent);
|
||||
}
|
||||
|
||||
.add-sessions-tile__icon {
|
||||
font-size: 24px;
|
||||
line-height: 1;
|
||||
}
|
||||
|
||||
.add-sessions-tile__label {
|
||||
font-size: 12px;
|
||||
}
|
||||
|
||||
@@ -4102,18 +4102,21 @@ def test_open_flyout_sheet_add_to_view_calls_view_picker() -> None:
|
||||
|
||||
|
||||
def test_open_flyout_submenu_filters_current_view() -> None:
|
||||
"""_openFlyoutSubmenu must skip the current user view from the submenu list.
|
||||
"""_openFlyoutSubmenu must show ALL user views (unified views submenu, Fix 4).
|
||||
|
||||
VIOLATION: When in a user view, showing that view in the 'Add to View' submenu
|
||||
was confusing — the user already has 'Remove from [ViewName]' for it.
|
||||
Fix 4: The old behavior filtered out the current view because 'Remove from [ViewName]'
|
||||
handled it. Now that 'Remove from [ViewName]' is removed, the submenu shows ALL
|
||||
views with toggle checkmarks so the user can multi-toggle without the submenu closing.
|
||||
"""
|
||||
fn_body = _JS.split("function _openFlyoutSubmenu")[1].split("\nfunction ")[0]
|
||||
assert "_activeView" in fn_body, (
|
||||
"_openFlyoutSubmenu must reference _activeView to filter the current view"
|
||||
# Must NOT have the old 'isInUserView' skip logic
|
||||
assert "isInUserView" not in fn_body, (
|
||||
"_openFlyoutSubmenu must not skip the current view — "
|
||||
"Fix 4: unified submenu shows ALL views with toggle checkmarks"
|
||||
)
|
||||
# The filter must specifically skip when in a user view (not 'all' or 'hidden')
|
||||
assert "isInUserView" in fn_body or "_activeView" in fn_body, (
|
||||
"_openFlyoutSubmenu must filter out the current user view from the submenu"
|
||||
# Must show checkmarks for views the session is in
|
||||
assert "\u2713" in fn_body or "isIn" in fn_body, (
|
||||
"_openFlyoutSubmenu must show checkmarks for views the session is already in"
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,490 @@
|
||||
"""Tests for 6 UX fixes: manage-view rename/delete, unified views submenu,
|
||||
stable checkbox list, dead code cleanup.
|
||||
"""
|
||||
|
||||
import pathlib
|
||||
import re
|
||||
|
||||
JS_PATH = pathlib.Path(__file__).parent.parent / "frontend" / "app.js"
|
||||
CSS_PATH = pathlib.Path(__file__).parent.parent / "frontend" / "style.css"
|
||||
|
||||
_JS: str = JS_PATH.read_text()
|
||||
_CSS: str = CSS_PATH.read_text()
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────────────────────────────
|
||||
# Fix 1: Manage View panel — rename click handler
|
||||
# ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_open_manage_view_panel_adds_rename_click_handler() -> None:
|
||||
"""openManageViewPanel must attach a click handler on #manage-view-name."""
|
||||
match = re.search(
|
||||
r"function openManageViewPanel\s*\(\s*\)\s*\{(.*?)(?=\nfunction |\n// )",
|
||||
_JS,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert match, "openManageViewPanel function not found"
|
||||
body = match.group(1)
|
||||
# Must reference manage-view-name and attach a click handler
|
||||
assert "manage-view-name" in body, (
|
||||
"openManageViewPanel must reference manage-view-name"
|
||||
)
|
||||
assert "onclick" in body or "addEventListener" in body or "click" in body.lower(), (
|
||||
"openManageViewPanel must attach a click handler on manage-view-name"
|
||||
)
|
||||
|
||||
|
||||
def test_manage_view_rename_creates_input_element() -> None:
|
||||
"""The rename handler on manage-view-name must create an <input> element."""
|
||||
# The click handler should create an input element for in-place renaming
|
||||
assert "manage-view-panel__name-input" in _JS or (
|
||||
"type" in _JS and "text" in _JS and "manage-view-name" in _JS
|
||||
), "Rename handler must create an input element for inline editing"
|
||||
|
||||
|
||||
def test_manage_view_rename_validates_non_empty() -> None:
|
||||
"""The rename handler must validate the name is non-empty before saving."""
|
||||
# Check that the rename logic in openManageViewPanel has non-empty validation
|
||||
# The validation logic should be present somewhere in the file
|
||||
assert "trim()" in _JS, "Rename must trim and validate non-empty input"
|
||||
|
||||
|
||||
def test_manage_view_rename_patches_settings() -> None:
|
||||
"""The rename flow must PATCH /api/settings with updated view name."""
|
||||
# The rename action must call PATCH /api/settings
|
||||
assert "PATCH" in _JS and "/api/settings" in _JS, (
|
||||
"Rename must PATCH /api/settings"
|
||||
)
|
||||
|
||||
|
||||
def test_manage_view_rename_handles_escape() -> None:
|
||||
"""The rename input must revert on Escape key."""
|
||||
match = re.search(
|
||||
r"function openManageViewPanel\s*\(\s*\)\s*\{(.*?)(?=\nfunction |\n// )",
|
||||
_JS,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert match, "openManageViewPanel function not found"
|
||||
body = match.group(1)
|
||||
assert "Escape" in body, (
|
||||
"openManageViewPanel rename handler must handle Escape key to revert"
|
||||
)
|
||||
|
||||
|
||||
def test_manage_view_rename_handles_enter() -> None:
|
||||
"""The rename input must commit on Enter key."""
|
||||
match = re.search(
|
||||
r"function openManageViewPanel\s*\(\s*\)\s*\{(.*?)(?=\nfunction |\n// )",
|
||||
_JS,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert match, "openManageViewPanel function not found"
|
||||
body = match.group(1)
|
||||
assert "Enter" in body, (
|
||||
"openManageViewPanel rename handler must handle Enter key to commit"
|
||||
)
|
||||
|
||||
|
||||
def test_manage_view_rename_updates_active_view() -> None:
|
||||
"""After rename, _activeView must be updated to the new name."""
|
||||
match = re.search(
|
||||
r"function openManageViewPanel\s*\(\s*\)\s*\{(.*?)(?=\nfunction |\n// )",
|
||||
_JS,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert match, "openManageViewPanel function not found"
|
||||
body = match.group(1)
|
||||
assert "_activeView" in body, (
|
||||
"openManageViewPanel rename handler must update _activeView to new name"
|
||||
)
|
||||
|
||||
|
||||
def test_manage_view_name_input_css_exists() -> None:
|
||||
""".manage-view-panel__name-input CSS rule must exist for the rename input."""
|
||||
assert ".manage-view-panel__name-input" in _CSS, (
|
||||
".manage-view-panel__name-input CSS rule must exist for the rename input"
|
||||
)
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────────────────────────────
|
||||
# Fix 2: Manage View panel — delete button in header
|
||||
# ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_open_manage_view_panel_has_delete_button() -> None:
|
||||
"""openManageViewPanel must render a delete/trash button in the header."""
|
||||
match = re.search(
|
||||
r"function openManageViewPanel\s*\(\s*\)\s*\{(.*?)(?=\nfunction |\n// )",
|
||||
_JS,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert match, "openManageViewPanel function not found"
|
||||
body = match.group(1)
|
||||
# Must have some delete/trash button or delete action
|
||||
has_delete = (
|
||||
"delete" in body.lower()
|
||||
or "trash" in body.lower()
|
||||
or "manage-view-delete" in body
|
||||
or "data-action=\"delete\"" in body
|
||||
)
|
||||
assert has_delete, (
|
||||
"openManageViewPanel must render a delete button in the header"
|
||||
)
|
||||
|
||||
|
||||
def test_manage_view_delete_shows_confirmation() -> None:
|
||||
"""The delete button must show inline confirmation before deleting."""
|
||||
match = re.search(
|
||||
r"function openManageViewPanel\s*\(\s*\)\s*\{(.*?)(?=\nfunction |\n// )",
|
||||
_JS,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert match, "openManageViewPanel function not found"
|
||||
body = match.group(1)
|
||||
# Must have Yes/No or confirm-delete pattern
|
||||
has_confirm = (
|
||||
"confirm" in body.lower()
|
||||
or "Yes" in body
|
||||
or "confirm-delete" in body
|
||||
or "Delete this view" in body
|
||||
)
|
||||
assert has_confirm, (
|
||||
"Manage View delete button must show inline confirmation (Yes/No) before deleting"
|
||||
)
|
||||
|
||||
|
||||
def test_manage_view_delete_removes_view_from_settings() -> None:
|
||||
"""Delete confirmation must PATCH /api/settings to remove the view."""
|
||||
# The delete flow must involve PATCH /api/settings with updated views
|
||||
# This is already present for the delete in settings tab, so check it's
|
||||
# also connected from openManageViewPanel
|
||||
match = re.search(
|
||||
r"function openManageViewPanel\s*\(\s*\)\s*\{(.*?)(?=\nfunction |\n// )",
|
||||
_JS,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert match, "openManageViewPanel function not found"
|
||||
body = match.group(1)
|
||||
# openManageViewPanel must either patch directly or call a helper that does
|
||||
assert "api(" in body or "PATCH" in body or "_saveViews" in body or "splice" in body, (
|
||||
"Manage View delete must call PATCH /api/settings to remove the view"
|
||||
)
|
||||
|
||||
|
||||
def test_manage_view_delete_switches_to_all() -> None:
|
||||
"""After deleting the view, must switch to 'all' view."""
|
||||
match = re.search(
|
||||
r"function openManageViewPanel\s*\(\s*\)\s*\{(.*?)(?=\nfunction |\n// )",
|
||||
_JS,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert match, "openManageViewPanel function not found"
|
||||
body = match.group(1)
|
||||
assert "switchView" in body or "'all'" in body, (
|
||||
"Manage View delete must switch to 'all' view after deletion"
|
||||
)
|
||||
|
||||
|
||||
def test_manage_view_delete_shows_toast() -> None:
|
||||
"""After confirming delete, must show a toast notification."""
|
||||
match = re.search(
|
||||
r"function openManageViewPanel\s*\(\s*\)\s*\{(.*?)(?=\nfunction |\n// )",
|
||||
_JS,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert match, "openManageViewPanel function not found"
|
||||
body = match.group(1)
|
||||
assert "showToast" in body, (
|
||||
"Manage View delete must call showToast after deleting"
|
||||
)
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────────────────────────────
|
||||
# Fix 3: Remove rename affordance from settings tab
|
||||
# ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_views_settings_name_no_cursor_pointer_css() -> None:
|
||||
""".views-settings-name must NOT have cursor: pointer (rename affordance removed)."""
|
||||
match = re.search(
|
||||
r"\.views-settings-name\s*\{([^}]*)\}",
|
||||
_CSS,
|
||||
re.DOTALL,
|
||||
)
|
||||
if match:
|
||||
body = match.group(1)
|
||||
assert "cursor: pointer" not in body, (
|
||||
".views-settings-name must not have cursor: pointer — rename is only via Manage panel"
|
||||
)
|
||||
|
||||
|
||||
def test_views_settings_name_no_hover_underline_css() -> None:
|
||||
""".views-settings-name:hover must NOT have text-decoration: underline (rename affordance removed)."""
|
||||
# Either the hover rule doesn't exist or it doesn't have underline
|
||||
hover_match = re.search(
|
||||
r"\.views-settings-name:hover\s*\{([^}]*)\}",
|
||||
_CSS,
|
||||
re.DOTALL,
|
||||
)
|
||||
if hover_match:
|
||||
body = hover_match.group(1)
|
||||
assert "text-decoration: underline" not in body, (
|
||||
".views-settings-name:hover must not have text-decoration: underline"
|
||||
)
|
||||
|
||||
|
||||
def test_views_settings_rename_input_css_removed() -> None:
|
||||
""".views-settings-rename-input CSS must be removed (dead class for settings-tab rename)."""
|
||||
assert ".views-settings-rename-input" not in _CSS, (
|
||||
".views-settings-rename-input CSS class must be removed — rename is no longer in settings tab"
|
||||
)
|
||||
|
||||
|
||||
def test_render_views_settings_tab_no_rename_handler() -> None:
|
||||
"""renderViewsSettingsTab must NOT attach a click-to-rename handler on name span."""
|
||||
match = re.search(
|
||||
r"function renderViewsSettingsTab\s*\(\s*\)\s*\{(.*?)(?=\nfunction |\n// |\n/\*)",
|
||||
_JS,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert match, "renderViewsSettingsTab function not found"
|
||||
body = match.group(1)
|
||||
# Must not have inline rename input creation
|
||||
assert "views-settings-rename-input" not in body, (
|
||||
"renderViewsSettingsTab must not create rename input — rename is only via Manage panel"
|
||||
)
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────────────────────────────
|
||||
# Fix 4: Merge Add/Remove into unified Views submenu
|
||||
# ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_flyout_menu_map_user_has_no_remove_from_view() -> None:
|
||||
"""FLYOUT_MENU_MAP 'user' entry must NOT have a separate 'remove-from-view' item."""
|
||||
# Find the FLYOUT_MENU_MAP definition
|
||||
match = re.search(
|
||||
r"const FLYOUT_MENU_MAP\s*=\s*\{(.*?)\};",
|
||||
_JS,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert match, "FLYOUT_MENU_MAP not found"
|
||||
map_body = match.group(1)
|
||||
# Find the 'user' section
|
||||
user_match = re.search(
|
||||
r"'user'\s*:\s*\[(.*?)\]",
|
||||
map_body,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert user_match, "'user' entry in FLYOUT_MENU_MAP not found"
|
||||
user_items = user_match.group(1)
|
||||
assert "remove-from-view" not in user_items, (
|
||||
"FLYOUT_MENU_MAP 'user' must not have separate 'remove-from-view' item — "
|
||||
"use unified Views submenu instead"
|
||||
)
|
||||
|
||||
|
||||
def test_flyout_menu_map_all_has_views_submenu() -> None:
|
||||
"""FLYOUT_MENU_MAP 'all' must have a Views submenu item."""
|
||||
match = re.search(
|
||||
r"const FLYOUT_MENU_MAP\s*=\s*\{(.*?)\};",
|
||||
_JS,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert match, "FLYOUT_MENU_MAP not found"
|
||||
map_body = match.group(1)
|
||||
all_match = re.search(
|
||||
r"'all'\s*:\s*\[(.*?)\]",
|
||||
map_body,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert all_match, "'all' entry in FLYOUT_MENU_MAP not found"
|
||||
all_items = all_match.group(1)
|
||||
# Should still have add-to-view or views-submenu action
|
||||
assert "add-to-view" in all_items or "views-submenu" in all_items, (
|
||||
"FLYOUT_MENU_MAP 'all' must have views submenu item"
|
||||
)
|
||||
|
||||
|
||||
def test_flyout_submenu_stays_open_after_toggle() -> None:
|
||||
"""_openFlyoutSubmenu must NOT close the submenu after a checkbox toggle."""
|
||||
match = re.search(
|
||||
r"function _openFlyoutSubmenu\s*\(.*?\)\s*\{(.*?)(?=\nfunction |\n// )",
|
||||
_JS,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert match, "_openFlyoutSubmenu function not found"
|
||||
body = match.group(1)
|
||||
# The submenu click handler should NOT call closeFlyoutMenu() on successful toggle
|
||||
# It should only call closeFlyoutMenu on 'new-view' action
|
||||
# Check that there's no unconditional closeFlyoutMenu after a toggle
|
||||
# The best we can check is that the submenu stays alive (no remove() on the submenu)
|
||||
# We verify the API PATCH callback doesn't close the menu
|
||||
assert "_flyoutSubmenuEl" in body or "submenu" in body.lower(), (
|
||||
"_openFlyoutSubmenu must maintain submenu reference for staying open"
|
||||
)
|
||||
|
||||
|
||||
def test_flyout_submenu_shows_checkmarks_for_current_view_sessions() -> None:
|
||||
"""_openFlyoutSubmenu must show checkmarks for views the session is already in."""
|
||||
match = re.search(
|
||||
r"function _openFlyoutSubmenu\s*\(.*?\)\s*\{(.*?)(?=\nfunction |\n// )",
|
||||
_JS,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert match, "_openFlyoutSubmenu function not found"
|
||||
body = match.group(1)
|
||||
# Must show a checkmark (✓) for views the session is in
|
||||
assert "\\u2713" in body or "✓" in body or "isIn" in body, (
|
||||
"_openFlyoutSubmenu must show checkmarks for views the session is already in"
|
||||
)
|
||||
|
||||
|
||||
def test_flyout_submenu_shows_all_user_views() -> None:
|
||||
"""_openFlyoutSubmenu must show ALL user views (not skip the current active view)."""
|
||||
match = re.search(
|
||||
r"function _openFlyoutSubmenu\s*\(.*?\)\s*\{(.*?)(?=\nfunction |\n// )",
|
||||
_JS,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert match, "_openFlyoutSubmenu function not found"
|
||||
body = match.group(1)
|
||||
# The old code skipped the active view: "if (isInUserView && v.name === _activeView) continue;"
|
||||
# Now it should NOT skip the current view — all views should be shown
|
||||
assert "isInUserView" not in body or "continue" not in body, (
|
||||
"_openFlyoutSubmenu must show ALL user views including the currently active one"
|
||||
)
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────────────────────────────
|
||||
# Fix 5: Fix checkbox re-render layout thrash
|
||||
# ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_manage_view_checkbox_no_full_rerender_on_success() -> None:
|
||||
"""renderManageViewList onChange handler must NOT call renderManageViewList() on success."""
|
||||
match = re.search(
|
||||
r"function renderManageViewList\s*\(\s*\)\s*\{(.*?)(?=\nfunction |\n// |\n/\*)",
|
||||
_JS,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert match, "renderManageViewList function not found"
|
||||
body = match.group(1)
|
||||
|
||||
# Find the listEl.onchange handler
|
||||
onchange_match = re.search(
|
||||
r"listEl\.onchange\s*=\s*function\s*\(e\)\s*\{(.*?)\}\s*;",
|
||||
body,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert onchange_match, "listEl.onchange handler not found in renderManageViewList"
|
||||
onchange_body = onchange_match.group(1)
|
||||
|
||||
# The .then() callback must NOT call renderManageViewList()
|
||||
then_match = re.search(
|
||||
r"\.then\s*\(function\s*\(\s*\)\s*\{(.*?)\}\s*\)",
|
||||
onchange_body,
|
||||
re.DOTALL,
|
||||
)
|
||||
if then_match:
|
||||
then_body = then_match.group(1)
|
||||
assert "renderManageViewList()" not in then_body, (
|
||||
"renderManageViewList onChange .then() must NOT call renderManageViewList() "
|
||||
"— this causes layout thrash and item position jumps"
|
||||
)
|
||||
|
||||
|
||||
def test_manage_view_checkbox_updates_summary_without_rerender() -> None:
|
||||
"""renderManageViewList onChange handler must update summary count in-place."""
|
||||
match = re.search(
|
||||
r"function renderManageViewList\s*\(\s*\)\s*\{(.*?)(?=\nfunction |\n// |\n/\*)",
|
||||
_JS,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert match, "renderManageViewList function not found"
|
||||
body = match.group(1)
|
||||
|
||||
# Find the listEl.onchange handler
|
||||
onchange_match = re.search(
|
||||
r"listEl\.onchange\s*=\s*function\s*\(e\)\s*\{(.*?)\}\s*;",
|
||||
body,
|
||||
re.DOTALL,
|
||||
)
|
||||
assert onchange_match, "listEl.onchange handler not found in renderManageViewList"
|
||||
onchange_body = onchange_match.group(1)
|
||||
|
||||
# Must update summary in the .then() callback
|
||||
then_match = re.search(
|
||||
r"\.then\s*\(function\s*\(\s*\)\s*\{(.*?)\}\s*\)",
|
||||
onchange_body,
|
||||
re.DOTALL,
|
||||
)
|
||||
if then_match:
|
||||
then_body = then_match.group(1)
|
||||
# Must update the summary count
|
||||
has_summary_update = (
|
||||
"summaryEl" in then_body
|
||||
or "manage-view-summary" in then_body
|
||||
or "textContent" in then_body
|
||||
)
|
||||
assert has_summary_update, (
|
||||
"renderManageViewList onChange .then() must update the summary count in-place"
|
||||
)
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────────────────────────────
|
||||
# Fix 6: Clean up dead CSS
|
||||
# ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_css_no_add_sessions_tile() -> None:
|
||||
""".add-sessions-tile CSS must be removed (dead code — old affordance tile)."""
|
||||
assert ".add-sessions-tile {" not in _CSS and ".add-sessions-tile{" not in _CSS, (
|
||||
".add-sessions-tile CSS must be removed — it's an orphaned class from old patterns"
|
||||
)
|
||||
|
||||
|
||||
def test_css_no_add_sessions_tile_icon() -> None:
|
||||
""".add-sessions-tile__icon CSS must be removed (dead code)."""
|
||||
assert ".add-sessions-tile__icon" not in _CSS, (
|
||||
".add-sessions-tile__icon CSS must be removed — orphaned class"
|
||||
)
|
||||
|
||||
|
||||
def test_css_no_add_sessions_tile_label() -> None:
|
||||
""".add-sessions-tile__label CSS must be removed (dead code)."""
|
||||
assert ".add-sessions-tile__label" not in _CSS, (
|
||||
".add-sessions-tile__label CSS must be removed — orphaned class"
|
||||
)
|
||||
|
||||
|
||||
def test_css_no_manage_view_panel_close_class() -> None:
|
||||
""".manage-view-panel__close CSS must be removed (HTML uses __close-btn)."""
|
||||
# The CSS has both __close and __close-btn — __close is orphaned
|
||||
# Only check for the specific orphaned rule (not __close-btn)
|
||||
# Look for the exact class name without the -btn suffix
|
||||
assert ".manage-view-panel__close {" not in _CSS, (
|
||||
".manage-view-panel__close CSS must be removed — HTML uses .manage-view-panel__close-btn"
|
||||
)
|
||||
|
||||
|
||||
def test_css_no_manage_view_panel_title_class() -> None:
|
||||
""".manage-view-panel__title CSS must be removed (HTML uses __name)."""
|
||||
assert ".manage-view-panel__title {" not in _CSS, (
|
||||
".manage-view-panel__title CSS must be removed — HTML uses .manage-view-panel__name"
|
||||
)
|
||||
|
||||
|
||||
def test_css_manage_view_panel_comment_updated() -> None:
|
||||
"""CSS comment must say 'Manage View Panel' not 'Add Sessions Panel'."""
|
||||
assert "Add Sessions Panel" not in _CSS, (
|
||||
"CSS comment 'Add Sessions Panel' must be updated to 'Manage View Panel'"
|
||||
)
|
||||
|
||||
|
||||
def test_css_manage_view_panel_name_input_exists() -> None:
|
||||
""".manage-view-panel__name-input CSS must exist (needed for Fix 1 rename input)."""
|
||||
assert ".manage-view-panel__name-input" in _CSS, (
|
||||
".manage-view-panel__name-input CSS must exist — it's used by the rename input in Fix 1"
|
||||
)
|
||||
Reference in New Issue
Block a user