fix: single close button on workspace panel, tooltip → 'Close'

- Remove duplicate mobile-close-btn from HTML
- Remove dead .mobile-close-btn CSS rules; unhide .close-preview at all viewports
- Change btnClearPreview tooltip from 'Hide workspace panel' to 'Close'
- Update tests across test_sprint41.py, test_sprint44.py, test_issue781.py,
  and test_mobile_layout.py to match new single-button model
This commit is contained in:
dobby-d-elf
2026-05-15 09:33:29 -06:00
parent acce80a50a
commit 0f86030f5f
7 changed files with 100 additions and 245 deletions
+1 -1
View File
@@ -201,7 +201,7 @@ function syncWorkspacePanelUI(){
const clearBtn=$('btnClearPreview');
if(clearBtn){
clearBtn.disabled=!isOpen;
_setButtonTooltip(clearBtn, hasPreview?'Close preview':'Hide workspace panel');
_setButtonTooltip(clearBtn, hasPreview?'Close preview':'Close');
if(!isCompact) clearBtn.style.display='';
}
}
-1
View File
@@ -1190,7 +1190,6 @@
<button class="panel-icon-btn has-tooltip has-tooltip--bottom" id="btnRefreshPanel" data-tooltip="Refresh" onclick="if(S.session)loadDir(S.currentDir)"><svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><polyline points="23 4 23 10 17 10"/><polyline points="1 20 1 14 7 14"/><path d="M3.51 9a9 9 0 0 1 14.85-3.36L23 10M1 14l4.64 4.36A9 9 0 0 0 20.49 15"/></svg></button>
<button class="panel-icon-btn has-tooltip has-tooltip--bottom" id="btnWorkspacePrefs" data-tooltip="Workspace options" data-i18n-title="workspace_options" aria-haspopup="true" aria-expanded="false" onclick="toggleWorkspacePrefsMenu(event)"><svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><circle cx="12" cy="5" r="1.5"/><circle cx="12" cy="12" r="1.5"/><circle cx="12" cy="19" r="1.5"/></svg><span class="workspace-prefs-dot" id="workspacePrefsDot" hidden></span></button>
<button class="panel-icon-btn close-preview has-tooltip has-tooltip--bottom" id="btnClearPreview" data-tooltip="Close preview"><svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><line x1="18" y1="6" x2="6" y2="18"/><line x1="6" y1="6" x2="18" y2="18"/></svg></button>
<button class="panel-icon-btn mobile-close-btn" onclick="handleWorkspaceClose()" title="Close" aria-label="Close workspace panel">×</button>
</div>
</div>
<div class="breadcrumb-bar" id="breadcrumbBar" style="display:none"></div>
-4
View File
@@ -1299,7 +1299,6 @@
@container rightpanel (max-width: 160px){
.workspace-panel-title-group{display:none;}
}
.mobile-close-btn{display:none;}
.panel-icon-btn{width:24px;height:24px;background:none;border:none;color:var(--muted);cursor:pointer;border-radius:5px;font-size:13px;display:flex;align-items:center;justify-content:center;transition:all .15s;}
.panel-icon-btn:hover{background:var(--hover-bg);color:var(--text);}
.panel-icon-btn:disabled{opacity:.35;cursor:not-allowed;}
@@ -1435,8 +1434,6 @@
@media(max-width:900px){
.rightpanel{display:none}
.workspace-toggle-btn,.mobile-files-btn{display:inline-flex!important;}
.mobile-close-btn{display:flex;}
.close-preview{display:none;}
}
@container composer-footer (max-width: 700px){
@@ -1538,7 +1535,6 @@
.rightpanel .resize-handle{display:none;}
.rightpanel .panel-header{row-gap:8px;}
.rightpanel .panel-actions{align-items:center;gap:8px;}
.rightpanel .mobile-close-btn{margin-left:auto;min-width:36px;min-height:36px;}
/* Topbar adjustments */
.topbar{padding:8px 12px;gap:8px;}
.topbar-title{font-size:14px;}
+22 -77
View File
@@ -1,12 +1,12 @@
"""
Tests for issue #781 — duplicate X close button in workspace preview header
on window resize below 900px breakpoint.
Tests for issue #781 — duplicate X close button in workspace preview header.
The fix: a single btnClearPreview (.close-preview) is the only close button,
visible on all devices. The mobile-close-btn element was removed entirely.
Verifies that:
- .close-preview is hidden (display:none) inside the @media (max-width:900px) block
- .mobile-close-btn is shown (display:flex) inside the same @media block
Both rules must appear inside the same @media(max-width:900px) block so that
at mobile widths only the mobile-close-btn is visible.
- .close-preview is NOT hidden by any media query (visible everywhere)
- .mobile-close-btn has no CSS rules remaining (element removed from HTML)
"""
import re
@@ -44,88 +44,33 @@ def _extract_media_block(css, media_query_pattern):
raise AssertionError("Unmatched brace in CSS after @media block")
def _strip_media_blocks(css):
"""Remove all @media {...} blocks from CSS, returning base rules only."""
result = []
i = 0
while i < len(css):
# Look for @media keyword
m = re.search(r"@media\b", css[i:])
if not m:
result.append(css[i:])
break
# Append everything before this @media
result.append(css[i : i + m.start()])
# Find the opening brace of this @media block
brace_start = css.index("{", i + m.start())
depth = 0
j = brace_start
while j < len(css):
if css[j] == "{":
depth += 1
elif css[j] == "}":
depth -= 1
if depth == 0:
i = j + 1
break
j += 1
else:
break
return "".join(result)
_MEDIA_900_PATTERN = r"@media\s*\(\s*max-width\s*:\s*900px\s*\)"
def test_mobile_close_btn_displayed_in_900px_block():
"""mobile-close-btn must be display:flex inside the 900px media query."""
def test_close_preview_not_hidden_in_900px_block():
"""The single close button (.close-preview) must NOT be hidden in any media query."""
css = _load_css()
block = _extract_media_block(css, _MEDIA_900_PATTERN)
assert ".mobile-close-btn" in block, (
".mobile-close-btn rule is missing from @media(max-width:900px) block"
)
rule_match = re.search(r"\.mobile-close-btn\s*\{([^}]*)\}", block)
assert rule_match, ".mobile-close-btn rule body not found in 900px block"
assert "display:flex" in rule_match.group(1).replace(" ", ""), (
".mobile-close-btn should have display:flex in the 900px media query"
assert ".close-preview" not in block, (
".close-preview must not appear in @media(max-width:900px) block"
"the single X button should be visible on all devices"
)
def test_close_preview_hidden_in_900px_block():
""".close-preview must be display:none inside the 900px media query (fix for #781)."""
def test_mobile_close_btn_not_in_css():
"""mobile-close-btn CSS rules should have been removed entirely."""
css = _load_css()
block = _extract_media_block(css, _MEDIA_900_PATTERN)
assert ".close-preview" in block, (
".close-preview rule is missing from @media(max-width:900px) block — "
"the duplicate-button fix (#781) may have been reverted"
)
rule_match = re.search(r"\.close-preview\s*\{([^}]*)\}", block)
assert rule_match, ".close-preview rule body not found in 900px block"
assert "display:none" in rule_match.group(1).replace(" ", ""), (
".close-preview should have display:none in the 900px media query to hide "
"the duplicate desktop X button at mobile widths"
assert ".mobile-close-btn" not in css, (
".mobile-close-btn CSS rule still present — the element was removed "
"from HTML so its styles should be cleaned up too"
)
def test_both_rules_in_same_media_block():
"""Both .close-preview and .mobile-close-btn must appear in the same 900px block."""
def test_close_preview_visible_in_base_css():
"""Outside media queries, .close-preview must NOT be display:none."""
css = _load_css()
block = _extract_media_block(css, _MEDIA_900_PATTERN)
assert ".mobile-close-btn" in block, (
".mobile-close-btn missing from @media(max-width:900px) block"
)
assert ".close-preview" in block, (
".close-preview missing from @media(max-width:900px) block"
)
def test_close_preview_visible_outside_media_query():
"""Outside the media query, .close-preview must NOT be display:none
(it should remain visible on desktop)."""
css = _load_css()
base_css = _strip_media_blocks(css)
close_rules = re.findall(r"\.close-preview\s*\{([^}]*)\}", base_css)
for rule_body in close_rules:
assert "display:none" not in rule_body.replace(" ", ""), (
".close-preview must not be hidden in base (desktop) CSS"
# Simple check: find all .close-preview rules and ensure none set display:none
for m in re.finditer(r"\.close-preview\s*\{([^}]*)\}", css):
assert "display:none" not in m.group(1).replace(" ", ""), (
".close-preview must not be hidden by any CSS rule"
)
-2
View File
@@ -207,8 +207,6 @@ def test_rightpanel_mobile_slide_over_css():
"open mobile rightpanel should keep the edge shadow"
assert re.search(r'\.rightpanel\s+\.panel-header\{[^}]*row-gap:\s*8px', rightpanel_block), \
"mobile workspace header should keep comfortable row spacing"
assert re.search(r'\.rightpanel\s+\.mobile-close-btn\{[^}]*margin-left:\s*auto', rightpanel_block), \
"mobile workspace close button should align to the far right"
def test_workspace_panel_inline_width_is_desktop_only():
+1 -56
View File
@@ -1,15 +1,11 @@
"""
Sprint 41 Tests: Title auto-generation fix + mobile close button CSS (PR #333).
Sprint 41 Tests: Title auto-generation fix (PR #333).
Covers:
- streaming.py: sessions titled 'New Chat' trigger auto-title generation
- streaming.py: sessions with empty/falsy title trigger auto-title generation
- streaming.py: sessions titled 'Untitled' (original guard) still trigger
- streaming.py: sessions with a user-set title do NOT trigger auto-title
- style.css: .mobile-close-btn is hidden by default (desktop rule present)
- style.css: .mobile-close-btn shown in <=900px media query
- style.css: #btnCollapseWorkspacePanel hidden in <=900px media query
- index.html: both .mobile-close-btn and #btnCollapseWorkspacePanel buttons exist
"""
import pathlib
import re
@@ -59,57 +55,6 @@ class TestTitleAutoGenerationCondition(unittest.TestCase):
"Expected at least 3 OR-joined sub-conditions (Untitled, New Chat, not s.title)")
# ── style.css: mobile close button visibility ─────────────────────────────
class TestMobileCloseButtonCSS(unittest.TestCase):
"""Verify CSS rules that control the duplicate close button on mobile."""
def test_mobile_close_btn_hidden_by_default(self):
"""Desktop default: .mobile-close-btn must be display:none outside any media query."""
# Find the rule before the first @media block that contains mobile-close-btn
# We look for the pattern in the desktop (non-media-query) section
self.assertIn(
".mobile-close-btn{display:none;}",
CSS.replace(" ", ""),
".mobile-close-btn should be hidden by default (desktop) — rule missing or wrong"
)
def test_mobile_close_btn_shown_in_900px_query(self):
"""Inside max-width:900px media query, .mobile-close-btn must be display:flex."""
# Extract the 900px media block
m = re.search(r'@media\s*\(max-width\s*:\s*900px\)\s*\{([^{}]*(?:\{[^{}]*\}[^{}]*)*)\}',
CSS)
self.assertIsNotNone(m, "@media(max-width:900px) block not found in style.css")
block = m.group(1).replace(" ", "")
self.assertIn(".mobile-close-btn{display:flex;}",
block,
".mobile-close-btn must be display:flex inside the 900px media query")
def test_900px_query_retains_existing_rules(self):
"""Ensure the PR didn't accidentally drop existing rules from the 900px block."""
m = re.search(r'@media\s*\(max-width\s*:\s*900px\)\s*\{([^{}]*(?:\{[^{}]*\}[^{}]*)*)\}',
CSS)
self.assertIsNotNone(m)
block = m.group(1)
self.assertIn("rightpanel", block, ".rightpanel rule missing from 900px block")
self.assertIn("mobile-files-btn", block, ".mobile-files-btn rule missing from 900px block")
# ── index.html: button presence ───────────────────────────────────────────
class TestWorkspacePanelButtons(unittest.TestCase):
"""Verify the workspace panel close control remains present."""
def test_mobile_close_button_exists(self):
self.assertIn("mobile-close-btn", HTML,
".mobile-close-btn button must exist in index.html")
def test_mobile_close_button_has_aria_label(self):
"""Accessibility: mobile close button must have an aria-label."""
m = re.search(r'class="[^"]*mobile-close-btn[^"]*"[^>]*>', HTML)
self.assertIsNotNone(m, "Could not find mobile-close-btn element")
self.assertIn("aria-label", m.group(0),
"mobile-close-btn must have aria-label for accessibility")
class TestIssue495TitleStreaming(unittest.TestCase):
+76 -104
View File
@@ -1,15 +1,13 @@
"""
Sprint 44 Tests: Workspace panel close button fixes (PR #413).
Sprint 44 Tests: Workspace panel close button (PR #413).
Covers:
- index.html: mobile-close-btn now calls handleWorkspaceClose() instead of
closeWorkspacePanel(), so hitting X while a file is open returns you to the
file browser rather than collapsing the whole panel.
- boot.js: syncWorkspacePanelUI() hides #btnClearPreview (the X icon) on
desktop when no file preview is open, eliminating the duplicate X that
appeared alongside the chevron collapse button.
- boot.js: handleWorkspaceClose() logic clears preview when one is visible,
closes panel otherwise (existing function, confirmed wired to both buttons).
- btnClearPreview is the single close button for all devices. Clicking it calls
handleWorkspaceClose(), which clears a file preview if open, or closes the
entire workspace panel otherwise.
- The mobile-close-btn element was removed only one X button remains.
- Tooltip text updated to "Close" (when no preview) and "Close preview"
(when a file is being viewed).
"""
import pathlib
import re
@@ -20,105 +18,79 @@ HTML = (REPO / "static" / "index.html").read_text(encoding="utf-8")
BOOT_JS = (REPO / "static" / "boot.js").read_text(encoding="utf-8")
class TestMobileCloseButtonBehavior(unittest.TestCase):
"""mobile-close-btn must call handleWorkspaceClose(), not closeWorkspacePanel()."""
class TestSingleCloseButton(unittest.TestCase):
"""btnClearPreview is the only close button, visible on all devices."""
def test_mobile_close_btn_calls_handle_workspace_close(self):
"""mobile-close-btn onclick must be handleWorkspaceClose(), not closeWorkspacePanel()."""
m = re.search(r'class="[^"]*mobile-close-btn[^"]*"[^>]*>', HTML)
self.assertIsNotNone(m, "mobile-close-btn element not found in index.html")
btn_html = m.group(0)
self.assertIn(
'onclick="handleWorkspaceClose()"',
btn_html,
"mobile-close-btn must call handleWorkspaceClose() so that hitting X "
"while a file is open closes the file first, not the whole panel",
)
def test_mobile_close_btn_does_not_call_close_workspace_panel_directly(self):
"""mobile-close-btn must NOT call closeWorkspacePanel() directly."""
m = re.search(r'class="[^"]*mobile-close-btn[^"]*"[^>]*>', HTML)
self.assertIsNotNone(m, "mobile-close-btn element not found in index.html")
btn_html = m.group(0)
self.assertNotIn(
'onclick="closeWorkspacePanel()"',
btn_html,
"mobile-close-btn must not call closeWorkspacePanel() directly — "
"it would bypass the two-step close logic and collapse the panel even "
"when a file is being viewed",
)
def test_handle_workspace_close_defined_in_boot_js(self):
"""handleWorkspaceClose() must be defined in boot.js."""
self.assertIn(
"function handleWorkspaceClose()",
BOOT_JS,
"handleWorkspaceClose() is missing from boot.js",
)
def test_handle_workspace_close_clears_preview_first(self):
"""handleWorkspaceClose() must call clearPreview() when a preview is visible."""
# The function must check for visible preview and call clearPreview
self.assertIn(
"clearPreview()",
BOOT_JS,
"handleWorkspaceClose() must call clearPreview() when preview is visible",
)
def test_handle_workspace_close_falls_back_to_close_panel(self):
"""handleWorkspaceClose() must call closeWorkspacePanel() as fallback."""
# Find the function start and extract until the closing brace by scanning
start = BOOT_JS.find("function handleWorkspaceClose()")
self.assertNotEqual(start, -1, "handleWorkspaceClose() not found in boot.js")
# Extract a generous window after the function start
fn_window = BOOT_JS[start : start + 400]
self.assertIn(
"closeWorkspacePanel()",
fn_window,
"handleWorkspaceClose() must call closeWorkspacePanel() as its fallback path",
)
class TestDesktopNoDuplicateXButton(unittest.TestCase):
"""On desktop, only one X/close control should appear at a time."""
def test_sync_workspace_panel_ui_hides_clear_preview_on_desktop(self):
"""syncWorkspacePanelUI() must set display:none on btnClearPreview when no preview and desktop."""
self.assertIn(
"clearBtn.style.display",
BOOT_JS,
"syncWorkspacePanelUI() must control clearBtn.style.display to hide it "
"on desktop when no file preview is open",
)
def test_clear_preview_hidden_when_no_preview(self):
"""The display toggle for btnClearPreview must key off hasPreview."""
# Expect something like: clearBtn.style.display=hasPreview?'':'none'
# or clearBtn.style.display = hasPreview ? '' : 'none'
pattern = r"clearBtn\.style\.display\s*=\s*hasPreview"
self.assertRegex(
BOOT_JS,
pattern,
"btnClearPreview display must be conditioned on hasPreview in "
"syncWorkspacePanelUI() to avoid a duplicate X on desktop",
)
def test_clear_preview_toggle_only_applied_on_desktop(self):
"""The display toggle must be guarded by !isCompact so mobile is unaffected."""
# Expect: if(!isCompact) clearBtn.style.display=...
pattern = r"isCompact.*clearBtn\.style\.display|clearBtn\.style\.display.*isCompact"
self.assertRegex(
BOOT_JS,
pattern,
"btnClearPreview display toggle must be guarded by isCompact so the "
"mobile X button visibility is not accidentally affected",
)
def test_btnclearpreview_exists_in_html(self):
"""#btnClearPreview must still exist in the HTML (not removed)."""
def test_btn_clear_preview_exists_in_html(self):
"""#btnClearPreview must exist in index.html."""
self.assertIn(
'id="btnClearPreview"',
HTML,
"#btnClearPreview must remain in index.html",
"#btnClearPreview must be present in index.html",
)
def test_mobile_close_btn_removed_from_html(self):
"""mobile-close-btn element should no longer exist in index.html."""
self.assertNotIn(
"mobile-close-btn",
HTML,
"mobile-close-btn was removed — only btnClearPreview remains as close control",
)
def test_btn_clear_preview_wired_to_handle_workspace_close(self):
"""btnClearPreview onclick must be handleWorkspaceClose."""
self.assertRegex(
BOOT_JS,
r"btnClearPreview.*onclick\s*=\s*handleWorkspaceClose",
"btnClearPreview must call handleWorkspaceClose so that clicking X "
"either clears a preview or closes the panel",
)
class TestHandleWorkspaceCloseLogic(unittest.TestCase):
"""handleWorkspaceClose() clears preview first, falls back to close panel."""
def test_function_defined(self):
self.assertIn(
"function handleWorkspaceClose()",
BOOT_JS,
"handleWorkspaceClose() must exist in boot.js",
)
def test_clears_preview_when_open(self):
idx = BOOT_JS.find("function handleWorkspaceClose()")
self.assertGreater(idx, 0, "handleWorkspaceClose() not found")
body = BOOT_JS[idx:idx + 500]
self.assertIn(
"clearPreview()",
body,
"handleWorkspaceClose() must call clearPreview() when a preview is open",
)
def test_falls_back_to_close_panel(self):
idx = BOOT_JS.find("function handleWorkspaceClose()")
self.assertGreater(idx, 0, "handleWorkspaceClose() not found")
body = BOOT_JS[idx:idx + 500]
self.assertIn(
"closeWorkspacePanel()",
body,
"handleWorkspaceClose() must call closeWorkspacePanel() as fallback",
)
class TestTooltipText(unittest.TestCase):
"""The X button tooltip says 'Close' when no preview, 'Close preview' otherwise."""
def test_tooltip_uses_close(self):
"""syncWorkspacePanelUI() sets tooltip to 'Close' (not 'Hide workspace panel')."""
idx = BOOT_JS.find("function syncWorkspacePanelUI()")
self.assertGreater(idx, 0, "syncWorkspacePanelUI() not found")
body = BOOT_JS[idx:idx + 2000]
# The tooltip line should contain 'Close' and NOT 'Hide workspace panel'
self.assertIn(
"'Close'",
body,
"btnClearPreview tooltip must use 'Close' instead of 'Hide workspace panel'",
)