fix: address Phase 3 COE verification findings — tile click guard, disclosure visibility, mobile confirm, dead code cleanup
This commit is contained in:
+155
-87
@@ -145,7 +145,7 @@ let _flyoutRemoteId = null;
|
||||
* { label, action, className?, separator? }
|
||||
* The 'user' view type gets the active view name injected at render time.
|
||||
*/
|
||||
var FLYOUT_MENU_MAP = {
|
||||
const FLYOUT_MENU_MAP = {
|
||||
'all': [
|
||||
{ label: 'Add to View\u2026', action: 'add-to-view', className: 'flyout-menu__item--has-submenu' },
|
||||
{ label: 'Hide', action: 'hide' },
|
||||
@@ -595,7 +595,6 @@ function buildSidebarHTML(session, currentSession) {
|
||||
`<div class="sidebar-item-header">` +
|
||||
`<span class="sidebar-item-name">${escapedName}</span>` +
|
||||
badgeHtml +
|
||||
`<button class="sidebar-delete" data-session="${escapedName}" aria-label="Kill session">×</button>` +
|
||||
`</div>` +
|
||||
`<div class="sidebar-item-body"><pre>${ansiToHtml(lastLines)}</pre></div>` +
|
||||
`</article>`
|
||||
@@ -734,8 +733,6 @@ function renderSidebar(sessions, currentSession) {
|
||||
const name = item.dataset.session;
|
||||
const remoteId = item.dataset.remoteId || '';
|
||||
on(item, 'click', (e) => {
|
||||
// Don't navigate when clicking the delete button inside the item
|
||||
if (e.target.closest && e.target.closest('.sidebar-delete')) return;
|
||||
if (name !== currentSession) openSession(name, { remoteId });
|
||||
});
|
||||
});
|
||||
@@ -1464,8 +1461,8 @@ function renderGrid(sessions) {
|
||||
// Bind interaction handlers on each tile
|
||||
document.querySelectorAll('.session-tile').forEach(function(tile) {
|
||||
on(tile, 'click', (e) => {
|
||||
// Don't navigate when clicking the delete button inside the tile
|
||||
if (e.target.closest && e.target.closest('.tile-delete')) return;
|
||||
// Don't navigate when clicking the options button inside the tile
|
||||
if (e.target.closest && e.target.closest('.tile-options-btn')) return;
|
||||
// Don't open error/status tiles (unreachable, auth_failed)
|
||||
if (tile.classList.contains('source-tile--error') || !tile.dataset.session) return;
|
||||
openSession(tile.dataset.session, { remoteId: tile.dataset.remoteId || '' });
|
||||
@@ -1658,7 +1655,7 @@ function _openFlyoutSheet() {
|
||||
var items = FLYOUT_MENU_MAP[viewType] || FLYOUT_MENU_MAP['all'];
|
||||
|
||||
var html = '<div class="flyout-sheet__backdrop"></div>';
|
||||
html += '<div class="flyout-sheet__panel">';
|
||||
html += '<div class="flyout-sheet__panel" aria-label="Session options" role="menu">';
|
||||
html += '<div class="flyout-sheet__handle" aria-hidden="true"></div>';
|
||||
|
||||
for (var i = 0; i < items.length; i++) {
|
||||
@@ -1678,7 +1675,7 @@ function _openFlyoutSheet() {
|
||||
var cls = 'flyout-sheet__item';
|
||||
if (item.className && item.className.indexOf('danger') !== -1) cls += ' flyout-sheet__item--danger';
|
||||
|
||||
html += '<button class="' + cls + '" data-action="' + item.action + '">';
|
||||
html += '<button class="' + cls + '" role="menuitem" data-action="' + item.action + '">';
|
||||
html += label;
|
||||
html += '</button>';
|
||||
}
|
||||
@@ -1707,13 +1704,18 @@ function _openFlyoutSheet() {
|
||||
|
||||
var action = btn.dataset.action;
|
||||
if (action === 'add-to-view' || action === 'unhide-add-to-view') {
|
||||
// On mobile, close sheet and open Add Sessions panel
|
||||
// On mobile, show a view picker sheet (not the Add Sessions panel which is for the active view)
|
||||
var sessionKey = _flyoutSessionKey;
|
||||
var sessionName = _flyoutSessionName;
|
||||
var unhideFirst = action === 'unhide-add-to-view';
|
||||
closeFlyoutMenu();
|
||||
openAddSessionsPanel();
|
||||
_openMobileViewPicker(sessionKey, sessionName, unhideFirst);
|
||||
} else if (action === 'kill') {
|
||||
// Simple confirm on mobile (inline doesn't work well in sheets)
|
||||
// Show a confirmation sheet — consistent with the existing sheet pattern
|
||||
var killName = _flyoutSessionName;
|
||||
var killRemoteId = _flyoutRemoteId;
|
||||
closeFlyoutMenu();
|
||||
killSession(_flyoutSessionName, _flyoutRemoteId);
|
||||
_openMobileKillConfirm(killName, killRemoteId);
|
||||
} else {
|
||||
// Dispatch directly
|
||||
_handleFlyoutClick(e);
|
||||
@@ -1722,6 +1724,144 @@ function _openFlyoutSheet() {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Show a confirmation bottom sheet before killing a session on mobile.
|
||||
* Shows "Kill [sessionName]?" with Kill and Cancel buttons.
|
||||
* @param {string} sessionName
|
||||
* @param {string} remoteId
|
||||
*/
|
||||
function _openMobileKillConfirm(sessionName, remoteId) {
|
||||
var sheet = document.createElement('div');
|
||||
sheet.className = 'flyout-sheet';
|
||||
|
||||
var html = '<div class="flyout-sheet__backdrop"></div>';
|
||||
html += '<div class="flyout-sheet__panel" aria-label="Confirm kill session" role="alertdialog">';
|
||||
html += '<div class="flyout-sheet__handle" aria-hidden="true"></div>';
|
||||
html += '<div class="flyout-sheet__title">Kill ' + escapeHtml(sessionName) + '?</div>';
|
||||
html += '<button class="flyout-sheet__item flyout-sheet__item--danger" data-action="confirm-kill" role="menuitem">Kill</button>';
|
||||
html += '<button class="flyout-sheet__item" data-action="cancel" role="menuitem">Cancel</button>';
|
||||
html += '</div>';
|
||||
|
||||
sheet.innerHTML = html;
|
||||
document.body.appendChild(sheet);
|
||||
|
||||
var backdrop = sheet.querySelector('.flyout-sheet__backdrop');
|
||||
if (backdrop) backdrop.addEventListener('click', function() { sheet.remove(); });
|
||||
|
||||
var panel = sheet.querySelector('.flyout-sheet__panel');
|
||||
if (panel) {
|
||||
panel.addEventListener('click', function(e) {
|
||||
var btn = e.target.closest('[data-action]');
|
||||
if (!btn) return;
|
||||
sheet.remove();
|
||||
if (btn.dataset.action === 'confirm-kill') {
|
||||
killSession(sessionName, remoteId);
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Open a bottom sheet listing all user views for the session (mobile view picker).
|
||||
* Same toggle behaviour as the desktop submenu — each tap fires a PATCH immediately.
|
||||
* The sheet has a "Done" button that closes it.
|
||||
* @param {string} sessionKey
|
||||
* @param {string} sessionName
|
||||
* @param {boolean} unhideFirst - If true, also unhide the session on first add
|
||||
*/
|
||||
function _openMobileViewPicker(sessionKey, sessionName, unhideFirst) {
|
||||
var views = (_serverSettings && _serverSettings.views) || [];
|
||||
if (views.length === 0) {
|
||||
showToast('No user views. Create one from the header dropdown.');
|
||||
return;
|
||||
}
|
||||
|
||||
var sheet = document.createElement('div');
|
||||
sheet.className = 'flyout-sheet';
|
||||
|
||||
var html = '<div class="flyout-sheet__backdrop"></div>';
|
||||
html += '<div class="flyout-sheet__panel" aria-label="Add to View" role="menu">';
|
||||
html += '<div class="flyout-sheet__handle" aria-hidden="true"></div>';
|
||||
|
||||
for (var i = 0; i < views.length; i++) {
|
||||
var v = views[i];
|
||||
var isIn = (v.sessions || []).indexOf(sessionKey) !== -1;
|
||||
html += '<button class="flyout-sheet__item" role="menuitem" data-view-index="' + i + '">';
|
||||
html += '<span style="margin-right:8px">' + (isIn ? '\u2713' : '\u00a0\u00a0') + '</span>';
|
||||
html += escapeHtml(v.name);
|
||||
html += '</button>';
|
||||
}
|
||||
|
||||
html += '<div class="flyout-sheet__separator"></div>';
|
||||
html += '<button class="flyout-sheet__item" data-action="done" role="menuitem">Done</button>';
|
||||
html += '</div>';
|
||||
|
||||
sheet.innerHTML = html;
|
||||
document.body.appendChild(sheet);
|
||||
|
||||
var backdrop = sheet.querySelector('.flyout-sheet__backdrop');
|
||||
if (backdrop) backdrop.addEventListener('click', function() { sheet.remove(); });
|
||||
|
||||
var panel = sheet.querySelector('.flyout-sheet__panel');
|
||||
if (panel) {
|
||||
panel.addEventListener('click', function(e) {
|
||||
var btn = e.target.closest('[data-action="done"]');
|
||||
if (btn) { sheet.remove(); return; }
|
||||
|
||||
var viewBtn = e.target.closest('[data-view-index]');
|
||||
if (!viewBtn) return;
|
||||
|
||||
var idx = parseInt(viewBtn.dataset.viewIndex, 10);
|
||||
var updatedViews = JSON.parse(JSON.stringify((_serverSettings && _serverSettings.views) || []));
|
||||
var view = updatedViews[idx];
|
||||
if (!view) return;
|
||||
|
||||
var sessions = view.sessions || [];
|
||||
var pos = sessions.indexOf(sessionKey);
|
||||
var nowIn;
|
||||
if (pos !== -1) {
|
||||
sessions.splice(pos, 1);
|
||||
nowIn = false;
|
||||
} else {
|
||||
sessions.push(sessionKey);
|
||||
nowIn = true;
|
||||
}
|
||||
view.sessions = sessions;
|
||||
|
||||
var patch = { views: updatedViews };
|
||||
if (unhideFirst && nowIn) {
|
||||
var hidden = (_serverSettings && _serverSettings.hidden_sessions) || [];
|
||||
var hiddenIdx = hidden.indexOf(sessionKey);
|
||||
if (hiddenIdx !== -1) {
|
||||
var updatedHidden = hidden.slice();
|
||||
updatedHidden.splice(hiddenIdx, 1);
|
||||
patch.hidden_sessions = updatedHidden;
|
||||
unhideFirst = false; // only unhide on first successful add
|
||||
}
|
||||
}
|
||||
|
||||
// Update checkmark immediately for responsiveness
|
||||
var checkEl = viewBtn.querySelector('span');
|
||||
if (checkEl) checkEl.textContent = nowIn ? '\u2713' : '\u00a0\u00a0';
|
||||
|
||||
api('PATCH', '/api/settings', patch)
|
||||
.then(function() {
|
||||
if (_serverSettings) {
|
||||
_serverSettings.views = updatedViews;
|
||||
if (patch.hidden_sessions) _serverSettings.hidden_sessions = patch.hidden_sessions;
|
||||
}
|
||||
if (patch.hidden_sessions) renderGrid(_currentSessions || []);
|
||||
})
|
||||
.catch(function(err) {
|
||||
showToast('Couldn\u2019t save \u2014 try again');
|
||||
// Revert checkmark
|
||||
if (checkEl) checkEl.textContent = nowIn ? '\u00a0\u00a0' : '\u2713';
|
||||
console.warn('[_openMobileViewPicker] PATCH failed:', err);
|
||||
});
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Click-outside handler for the flyout menu.
|
||||
* @param {MouseEvent} e
|
||||
@@ -1788,9 +1928,12 @@ function _openFlyoutSubmenu(triggerItem, unhideFirst) {
|
||||
}
|
||||
|
||||
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';
|
||||
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>';
|
||||
@@ -2232,25 +2375,6 @@ function renderAddSessionsList() {
|
||||
}
|
||||
};
|
||||
|
||||
// Show/hide disclosure on hover for hidden items
|
||||
listEl.onmouseover = function(e) {
|
||||
var item = e.target.closest('.add-sessions-item--hidden');
|
||||
if (item) {
|
||||
var disc = item.nextElementSibling;
|
||||
if (disc && disc.classList.contains('add-sessions-item__disclosure')) {
|
||||
disc.style.display = '';
|
||||
}
|
||||
}
|
||||
};
|
||||
listEl.onmouseout = function(e) {
|
||||
var item = e.target.closest('.add-sessions-item--hidden');
|
||||
if (item) {
|
||||
var disc = item.nextElementSibling;
|
||||
if (disc && disc.classList.contains('add-sessions-item__disclosure')) {
|
||||
disc.style.display = 'none';
|
||||
}
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -2268,61 +2392,6 @@ function _getDeviceDisplayName(session) {
|
||||
return '';
|
||||
}
|
||||
|
||||
/**
|
||||
* Add a session to the active user view via PATCH.
|
||||
* If the session is hidden, also unhide it.
|
||||
* On error: show toast, revert checkbox.
|
||||
* @param {string} sessionKey
|
||||
* @param {boolean} unhideFirst
|
||||
* @param {HTMLInputElement} checkbox
|
||||
*/
|
||||
function _addSessionToActiveView(sessionKey, unhideFirst, checkbox) {
|
||||
var views = (_serverSettings && _serverSettings.views) || [];
|
||||
var updatedViews = JSON.parse(JSON.stringify(views));
|
||||
|
||||
// Find active view and add session
|
||||
for (var i = 0; i < updatedViews.length; i++) {
|
||||
if (updatedViews[i].name === _activeView) {
|
||||
var sessions = updatedViews[i].sessions || [];
|
||||
if (sessions.indexOf(sessionKey) === -1) {
|
||||
sessions.push(sessionKey);
|
||||
}
|
||||
updatedViews[i].sessions = sessions;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
var patch = { views: updatedViews };
|
||||
|
||||
if (unhideFirst) {
|
||||
var hidden = (_serverSettings && _serverSettings.hidden_sessions) || [];
|
||||
var hiddenIdx = hidden.indexOf(sessionKey);
|
||||
if (hiddenIdx !== -1) {
|
||||
var updatedHidden = hidden.slice();
|
||||
updatedHidden.splice(hiddenIdx, 1);
|
||||
patch.hidden_sessions = updatedHidden;
|
||||
}
|
||||
}
|
||||
|
||||
api('PATCH', '/api/settings', patch)
|
||||
.then(function() {
|
||||
if (_serverSettings) {
|
||||
_serverSettings.views = updatedViews;
|
||||
if (patch.hidden_sessions) _serverSettings.hidden_sessions = patch.hidden_sessions;
|
||||
}
|
||||
// Re-render the list (session is now in the view, so it disappears from the list)
|
||||
renderAddSessionsList();
|
||||
// Refresh the grid behind the panel
|
||||
renderGrid(_currentSessions || []);
|
||||
})
|
||||
.catch(function(err) {
|
||||
showToast('Couldn’t save — try again');
|
||||
if (checkbox) checkbox.checked = false;
|
||||
console.warn('[_addSessionToActiveView] PATCH failed:', err);
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
// ─── Notification permission ────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
@@ -3649,7 +3718,6 @@ function bindStaticEventListeners() {
|
||||
document.addEventListener('click', function(e) {
|
||||
var optionsBtn = e.target.closest && e.target.closest('.tile-options-btn');
|
||||
if (!optionsBtn) return;
|
||||
e.stopPropagation();
|
||||
openFlyoutMenu(optionsBtn);
|
||||
});
|
||||
|
||||
|
||||
@@ -1156,40 +1156,6 @@ body {
|
||||
|
||||
|
||||
|
||||
/* ============================================================
|
||||
Delete (kill) session buttons — appear on hover
|
||||
============================================================ */
|
||||
|
||||
.tile-delete {
|
||||
position: absolute;
|
||||
top: 8px;
|
||||
right: 8px;
|
||||
background: none;
|
||||
border: none;
|
||||
color: var(--text-dim);
|
||||
font-size: 14px;
|
||||
cursor: pointer;
|
||||
padding: 2px 6px;
|
||||
border-radius: 4px;
|
||||
opacity: 0;
|
||||
transition: opacity 150ms ease;
|
||||
pointer-events: none;
|
||||
z-index: 2;
|
||||
line-height: 1;
|
||||
}
|
||||
|
||||
.session-tile:hover .tile-delete,
|
||||
.session-tile:focus-within .tile-delete {
|
||||
opacity: 1;
|
||||
pointer-events: auto;
|
||||
}
|
||||
|
||||
.tile-delete:hover {
|
||||
color: #ef4444;
|
||||
background: rgba(239, 68, 68, 0.1);
|
||||
opacity: 1;
|
||||
}
|
||||
|
||||
.sidebar-delete {
|
||||
background: none;
|
||||
border: none;
|
||||
@@ -2005,7 +1971,6 @@ body {
|
||||
font-size: 11px;
|
||||
color: var(--text-dim);
|
||||
font-style: italic;
|
||||
display: none;
|
||||
}
|
||||
|
||||
/* Mobile: full-screen sheet for Add Sessions */
|
||||
|
||||
+30
-14
@@ -1354,10 +1354,15 @@ def test_instance_info_includes_device_id(client, tmp_path, monkeypatch):
|
||||
|
||||
def test_create_session_returns_200_with_name(client, monkeypatch):
|
||||
"""POST /api/sessions with valid name returns 200 with {name: name}."""
|
||||
from unittest.mock import MagicMock
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
mock_popen = MagicMock()
|
||||
monkeypatch.setattr("muxplex.main.subprocess.Popen", mock_popen)
|
||||
mock_proc = MagicMock()
|
||||
mock_proc.communicate = AsyncMock(return_value=(b"", b""))
|
||||
mock_proc.returncode = 0
|
||||
|
||||
monkeypatch.setattr(
|
||||
"muxplex.main.asyncio.create_subprocess_shell", AsyncMock(return_value=mock_proc)
|
||||
)
|
||||
|
||||
response = client.post("/api/sessions", json={"name": "my-project"})
|
||||
assert response.status_code == 200
|
||||
@@ -1368,6 +1373,7 @@ def test_create_session_returns_200_with_name(client, monkeypatch):
|
||||
def test_create_session_substitutes_name_in_template(client, tmp_path, monkeypatch):
|
||||
"""POST /api/sessions substitutes {name} with actual name in new_session_template."""
|
||||
import json
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import muxplex.settings as settings_mod
|
||||
|
||||
@@ -1375,18 +1381,22 @@ def test_create_session_substitutes_name_in_template(client, tmp_path, monkeypat
|
||||
monkeypatch.setattr(settings_mod, "SETTINGS_PATH", settings_path)
|
||||
settings_path.write_text(json.dumps({"new_session_template": "echo {name}"}))
|
||||
|
||||
popen_calls = []
|
||||
shell_calls = []
|
||||
|
||||
def mock_popen(cmd, **kwargs):
|
||||
popen_calls.append(cmd)
|
||||
return object()
|
||||
mock_proc = MagicMock()
|
||||
mock_proc.communicate = AsyncMock(return_value=(b"", b""))
|
||||
mock_proc.returncode = 0
|
||||
|
||||
monkeypatch.setattr("muxplex.main.subprocess.Popen", mock_popen)
|
||||
async def mock_create_subprocess(cmd, **kwargs):
|
||||
shell_calls.append(cmd)
|
||||
return mock_proc
|
||||
|
||||
monkeypatch.setattr("muxplex.main.asyncio.create_subprocess_shell", mock_create_subprocess)
|
||||
|
||||
response = client.post("/api/sessions", json={"name": "my-project"})
|
||||
assert response.status_code == 200
|
||||
assert len(popen_calls) == 1
|
||||
assert popen_calls[0] == "echo my-project"
|
||||
assert len(shell_calls) == 1
|
||||
assert shell_calls[0] == "echo my-project"
|
||||
|
||||
|
||||
def test_create_session_rejects_empty_name(client):
|
||||
@@ -2447,16 +2457,22 @@ def test_delete_session_logs_command_at_info(client, monkeypatch, tmp_path, capl
|
||||
def test_create_session_logs_command(client, monkeypatch, tmp_path, caplog):
|
||||
"""POST /api/sessions must log the command being launched at INFO level."""
|
||||
import logging
|
||||
from unittest.mock import MagicMock, patch
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import muxplex.settings as settings_mod
|
||||
|
||||
monkeypatch.setattr(settings_mod, "SETTINGS_PATH", tmp_path / "no-settings.json")
|
||||
|
||||
mock_proc = MagicMock()
|
||||
mock_proc.communicate = AsyncMock(return_value=(b"", b""))
|
||||
mock_proc.returncode = 0
|
||||
|
||||
monkeypatch.setattr(
|
||||
"muxplex.main.asyncio.create_subprocess_shell", AsyncMock(return_value=mock_proc)
|
||||
)
|
||||
|
||||
with caplog.at_level(logging.INFO, logger="muxplex.main"):
|
||||
with patch("muxplex.main.subprocess.Popen") as mock_popen:
|
||||
mock_popen.return_value = MagicMock()
|
||||
client.post("/api/sessions", json={"name": "new-session"})
|
||||
client.post("/api/sessions", json={"name": "new-session"})
|
||||
|
||||
log_messages = "\n".join(caplog.messages)
|
||||
assert "new-session" in log_messages, (
|
||||
|
||||
@@ -2224,3 +2224,53 @@ def test_add_sessions_item_styled() -> None:
|
||||
"""style.css must contain .add-sessions-item styles."""
|
||||
css = read_css()
|
||||
assert ".add-sessions-item" in css, "style.css must style .add-sessions-item"
|
||||
assert ".add-sessions-item" in css, "style.css must style .add-sessions-item"
|
||||
|
||||
|
||||
# ─── Phase 3 COE findings regression tests ──────────────────────────────────
|
||||
|
||||
|
||||
# ── BUG 2: disclosure is statically visible (not hover-gated) ────────────────
|
||||
|
||||
|
||||
def test_disclosure_not_hidden_by_css() -> None:
|
||||
"""`.add-sessions-item__disclosure` must NOT have `display: none` in style.css.
|
||||
|
||||
BUG: The CSS had `display: none` on the disclosure. The JS hover handlers set
|
||||
`disc.style.display = ''` (removing the inline style) which did NOT override the
|
||||
CSS rule — the disclosure remained invisible on hover. Fix: remove `display: none`
|
||||
from CSS so the disclosure is statically visible for hidden items (it is already
|
||||
only rendered for hidden items in the HTML).
|
||||
"""
|
||||
css = read_css()
|
||||
import re as _re
|
||||
|
||||
match = _re.search(
|
||||
r"\.add-sessions-item__disclosure\s*\{([^}]*)\}",
|
||||
css,
|
||||
_re.DOTALL,
|
||||
)
|
||||
assert match, ".add-sessions-item__disclosure rule not found in style.css"
|
||||
body = match.group(1)
|
||||
assert "display: none" not in body and "display:none" not in body.replace(" ", ""), (
|
||||
".add-sessions-item__disclosure must NOT have display:none — "
|
||||
"the disclosure must be statically visible for hidden items (BUG 2 fix). "
|
||||
"The hover-based show/hide was broken: setting disc.style.display='' doesn't "
|
||||
"override a CSS display:none rule — it only removes the inline style."
|
||||
)
|
||||
|
||||
|
||||
# ── CLEANUP 1: .tile-delete CSS removed ──────────────────────────────────────
|
||||
|
||||
|
||||
def test_tile_delete_css_removed() -> None:
|
||||
"""`.tile-delete` CSS rules must be removed from style.css.
|
||||
|
||||
CLEANUP: The .tile-delete button was removed from buildTileHTML (Phase 2 task).
|
||||
The orphaned CSS rules must be cleaned up.
|
||||
"""
|
||||
css = read_css()
|
||||
assert ".tile-delete" not in css, (
|
||||
".tile-delete CSS rules must be removed from style.css — "
|
||||
"the button was removed from buildTileHTML; orphaned CSS is dead code"
|
||||
)
|
||||
|
||||
@@ -3964,3 +3964,220 @@ def test_render_grid_closes_stale_flyout() -> None:
|
||||
assert "_flyoutSessionKey" in fn_body or "closeFlyoutMenu" in fn_body, (
|
||||
"renderGrid must check if the flyout's target session still exists and close if not"
|
||||
)
|
||||
|
||||
|
||||
# ─── Phase 3 COE findings regression tests ──────────────────────────────────
|
||||
|
||||
|
||||
# ── BUG 1: tile click handler guards .tile-options-btn ───────────────────────
|
||||
|
||||
|
||||
def test_tile_click_handler_guards_options_btn() -> None:
|
||||
"""Tile click handler early return must guard .tile-options-btn (not .tile-delete).
|
||||
|
||||
BUG: Previously the guard checked .tile-delete which was removed in Phase 3.
|
||||
Clicking ⋮ triggered BOTH flyout opening AND openSession() navigation.
|
||||
Fix: guard must check .tile-options-btn to stop event from reaching openSession().
|
||||
"""
|
||||
# The tile click handler is inside renderGrid — find it
|
||||
render_grid_body = _JS.split("function renderGrid")[1].split("\nfunction ")[0]
|
||||
assert "tile-options-btn" in render_grid_body, (
|
||||
"Tile click handler must guard against .tile-options-btn clicks — "
|
||||
"clicking ⋮ must NOT trigger openSession()"
|
||||
)
|
||||
# Confirm the old broken guard is gone
|
||||
assert "'tile-delete'" not in render_grid_body and '"tile-delete"' not in render_grid_body or (
|
||||
"tile-options-btn" in render_grid_body
|
||||
), (
|
||||
"Guard must use .tile-options-btn, not the old .tile-delete which was removed"
|
||||
)
|
||||
|
||||
|
||||
def test_flyout_delegation_handler_no_stop_propagation() -> None:
|
||||
"""bindStaticEventListeners .tile-options-btn delegation must NOT call e.stopPropagation().
|
||||
|
||||
BUG: e.stopPropagation() at document level is a no-op (document is the top of the
|
||||
bubble chain). It created a false sense of correctness while doing nothing.
|
||||
"""
|
||||
# Extract the tile-options-btn delegation handler block from bindStaticEventListeners
|
||||
bind_body = _JS.split("function bindStaticEventListeners")[1].split("\nfunction ")[0]
|
||||
# Find the section with tile-options-btn
|
||||
assert "tile-options-btn" in bind_body, (
|
||||
"bindStaticEventListeners must have .tile-options-btn delegation"
|
||||
)
|
||||
# The delegation block should not have stopPropagation
|
||||
tile_opts_section = bind_body.split("tile-options-btn")[1].split("});")[0]
|
||||
assert "stopPropagation" not in tile_opts_section, (
|
||||
"The .tile-options-btn delegation handler must not call e.stopPropagation() — "
|
||||
"it is a no-op at document level and creates false sense of correctness"
|
||||
)
|
||||
|
||||
|
||||
# ── BUG 2: disclosure has no hover-based show/hide in renderAddSessionsList ──
|
||||
|
||||
|
||||
def test_render_add_sessions_list_no_hover_disclosure() -> None:
|
||||
"""renderAddSessionsList must NOT have mouseover/mouseout handlers for disclosure.
|
||||
|
||||
BUG: The disclosure was shown only on hover via JS mouseover/mouseout. This broke
|
||||
on mobile (no hover) and was inconsistent. Fix: disclose statically — remove CSS
|
||||
display:none and remove the hover handlers. The disclosure appears whenever the
|
||||
hidden item is in the HTML (it's already conditionally rendered for hidden items).
|
||||
"""
|
||||
fn_body = _JS.split("function renderAddSessionsList")[1].split("\nfunction ")[0]
|
||||
assert "onmouseover" not in fn_body, (
|
||||
"renderAddSessionsList must not use onmouseover to show disclosure — "
|
||||
"the disclosure must be statically visible (BUG 2 fix)"
|
||||
)
|
||||
assert "onmouseout" not in fn_body, (
|
||||
"renderAddSessionsList must not use onmouseout to hide disclosure — "
|
||||
"the disclosure must be statically visible (BUG 2 fix)"
|
||||
)
|
||||
|
||||
|
||||
# ── BUG 3: mobile kill has confirmation sheet ─────────────────────────────────
|
||||
|
||||
|
||||
def test_open_mobile_kill_confirm_function_exists() -> None:
|
||||
"""app.js must define _openMobileKillConfirm for mobile kill confirmation.
|
||||
|
||||
BUG: Mobile kill previously called killSession() directly with no confirmation.
|
||||
Fix: A second bottom sheet confirms before killing.
|
||||
"""
|
||||
assert "function _openMobileKillConfirm" in _JS, (
|
||||
"_openMobileKillConfirm must be defined — mobile kill needs a confirmation sheet"
|
||||
)
|
||||
|
||||
|
||||
def test_open_flyout_sheet_kill_calls_confirm_not_direct() -> None:
|
||||
"""_openFlyoutSheet must call _openMobileKillConfirm, not killSession() directly.
|
||||
|
||||
BUG: Direct killSession() call gave no confirmation chance to the user on mobile.
|
||||
"""
|
||||
fn_body = _JS.split("function _openFlyoutSheet")[1].split("\nfunction ")[0]
|
||||
assert "_openMobileKillConfirm" in fn_body, (
|
||||
"Mobile kill action in _openFlyoutSheet must call _openMobileKillConfirm, not killSession() directly"
|
||||
)
|
||||
|
||||
|
||||
# ── VIOLATION 1: mobile Add to View opens view picker ────────────────────────
|
||||
|
||||
|
||||
def test_open_mobile_view_picker_function_exists() -> None:
|
||||
"""app.js must define _openMobileViewPicker for mobile view selection.
|
||||
|
||||
VIOLATION: Mobile 'Add to View...' opened openAddSessionsPanel() which is the
|
||||
session picker for the current view — wrong behaviour. Fix: a view picker that
|
||||
lists all user views with checkboxes.
|
||||
"""
|
||||
assert "function _openMobileViewPicker" in _JS, (
|
||||
"_openMobileViewPicker must be defined — mobile 'Add to View...' must list views, not sessions"
|
||||
)
|
||||
|
||||
|
||||
def test_open_flyout_sheet_add_to_view_calls_view_picker() -> None:
|
||||
"""_openFlyoutSheet must call _openMobileViewPicker, not openAddSessionsPanel.
|
||||
|
||||
VIOLATION: openAddSessionsPanel() is the session picker for the current view,
|
||||
not a view picker for the current session.
|
||||
"""
|
||||
fn_body = _JS.split("function _openFlyoutSheet")[1].split("\nfunction ")[0]
|
||||
assert "_openMobileViewPicker" in fn_body, (
|
||||
"Mobile 'Add to View' must call _openMobileViewPicker, not openAddSessionsPanel"
|
||||
)
|
||||
# Confirm the old wrong call is gone from the mobile handler
|
||||
# (openAddSessionsPanel may still exist for the grid affordance — just not here)
|
||||
assert "openAddSessionsPanel" not in fn_body.split("_openMobileViewPicker")[0].split(
|
||||
"action === 'add-to-view'"
|
||||
)[-1], (
|
||||
"_openFlyoutSheet must not call openAddSessionsPanel for the add-to-view action"
|
||||
)
|
||||
|
||||
|
||||
# ── VIOLATION 2: submenu filters current view ────────────────────────────────
|
||||
|
||||
|
||||
def test_open_flyout_submenu_filters_current_view() -> None:
|
||||
"""_openFlyoutSubmenu must skip the current user view from the submenu list.
|
||||
|
||||
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.
|
||||
"""
|
||||
fn_body = _JS.split("function _openFlyoutSubmenu")[1].split("\nfunction ")[0]
|
||||
assert "_activeView" in fn_body, (
|
||||
"_openFlyoutSubmenu must reference _activeView to filter the current view"
|
||||
)
|
||||
# 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"
|
||||
)
|
||||
|
||||
|
||||
# ── CLEANUP 2: sidebar kill button removed ────────────────────────────────────
|
||||
|
||||
|
||||
def test_build_sidebar_html_no_sidebar_delete_button() -> None:
|
||||
"""buildSidebarHTML must NOT include the .sidebar-delete kill button.
|
||||
|
||||
CLEANUP: The sidebar had a second kill location; flyout is now the only location.
|
||||
"""
|
||||
fn_body = _JS.split("function buildSidebarHTML")[1].split("\nfunction ")[0]
|
||||
assert "sidebar-delete" not in fn_body, (
|
||||
"buildSidebarHTML must not render .sidebar-delete button — "
|
||||
"kill is available only via the flyout menu (single kill location)"
|
||||
)
|
||||
|
||||
|
||||
# ── CLEANUP 3: dead _addSessionToActiveView removed ───────────────────────────
|
||||
|
||||
|
||||
def test_add_session_to_active_view_removed() -> None:
|
||||
"""_addSessionToActiveView must be removed — renderAddSessionsList does same inline.
|
||||
|
||||
CLEANUP: The function was fully implemented but never called. Dead code removed.
|
||||
"""
|
||||
assert "_addSessionToActiveView" not in _JS, (
|
||||
"_addSessionToActiveView must be removed — it was dead code never called; "
|
||||
"renderAddSessionsList handles the same logic inline"
|
||||
)
|
||||
|
||||
|
||||
# ── CLEANUP 4: ARIA on mobile flyout sheet ────────────────────────────────────
|
||||
|
||||
|
||||
def test_flyout_sheet_panel_has_aria_label() -> None:
|
||||
"""_openFlyoutSheet must add aria-label='Session options' to the sheet panel.
|
||||
|
||||
CLEANUP: The panel had no ARIA labelling for screen readers.
|
||||
"""
|
||||
fn_body = _JS.split("function _openFlyoutSheet")[1].split("\nfunction ")[0]
|
||||
assert "aria-label" in fn_body and "Session options" in fn_body, (
|
||||
"_openFlyoutSheet must set aria-label='Session options' on the sheet panel"
|
||||
)
|
||||
|
||||
|
||||
def test_flyout_sheet_items_have_role_menuitem() -> None:
|
||||
"""_openFlyoutSheet must add role='menuitem' to each sheet item button.
|
||||
|
||||
CLEANUP: Sheet items lacked ARIA role, breaking screen reader navigation.
|
||||
"""
|
||||
fn_body = _JS.split("function _openFlyoutSheet")[1].split("\nfunction ")[0]
|
||||
assert "role=\"menuitem\"" in fn_body or "role='menuitem'" in fn_body, (
|
||||
"_openFlyoutSheet must set role='menuitem' on each sheet item button"
|
||||
)
|
||||
|
||||
|
||||
# ── CLEANUP 5: FLYOUT_MENU_MAP is const ──────────────────────────────────────
|
||||
|
||||
|
||||
def test_flyout_menu_map_is_const() -> None:
|
||||
"""FLYOUT_MENU_MAP must be declared with const, not var.
|
||||
|
||||
CLEANUP: It is an immutable data structure; var was incorrect.
|
||||
"""
|
||||
assert "const FLYOUT_MENU_MAP" in _JS, (
|
||||
"FLYOUT_MENU_MAP must be declared with 'const', not 'var' — it is immutable"
|
||||
)
|
||||
assert "var FLYOUT_MENU_MAP" not in _JS, (
|
||||
"FLYOUT_MENU_MAP must not use 'var' — change to 'const'"
|
||||
)
|
||||
|
||||
@@ -434,13 +434,13 @@ def test_concurrent_ws_sessions(monkeypatch):
|
||||
|
||||
|
||||
def test_federation_ws_proxy_route_exists():
|
||||
"""App must have a WebSocket route at /federation/{remote_id}/terminal/ws."""
|
||||
"""App must have a WebSocket route at /federation/{device_id}/terminal/ws."""
|
||||
from starlette.routing import WebSocketRoute
|
||||
|
||||
ws_routes = [r for r in app.routes if isinstance(r, WebSocketRoute)]
|
||||
paths = [r.path for r in ws_routes]
|
||||
assert "/federation/{remote_id}/terminal/ws" in paths, (
|
||||
"App must have a WebSocket route at /federation/{remote_id}/terminal/ws"
|
||||
assert "/federation/{device_id}/terminal/ws" in paths, (
|
||||
"App must have a WebSocket route at /federation/{device_id}/terminal/ws"
|
||||
)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user