mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-26 11:40:26 +00:00
Stage 312: PR #1794 — fix(ux): rail tooltips + new-conversation clipping + context-menu hover + rename pre-fill by @nesquena-hermes
This commit is contained in:
nesquena-hermes
+1
-1
@@ -122,7 +122,7 @@
|
||||
<div class="panel-head">
|
||||
<span data-i18n="tab_chat">Chat</span>
|
||||
<div class="panel-head-actions">
|
||||
<button class="panel-head-btn has-tooltip has-tooltip--bottom" id="btnNewChat" data-tooltip="New conversation (Cmd+K)" data-i18n-title="new_conversation" aria-label="New conversation">
|
||||
<button class="panel-head-btn has-tooltip has-tooltip--bottom-right" id="btnNewChat" data-tooltip="New conversation (Cmd+K)" data-i18n-title="new_conversation" aria-label="New conversation">
|
||||
<svg viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><line x1="12" y1="5" x2="12" y2="19"/><line x1="5" y1="12" x2="19" y2="12"/></svg>
|
||||
</button>
|
||||
</div>
|
||||
|
||||
+2
-2
@@ -2884,7 +2884,7 @@ function _showProjectContextMenu(e, proj, chip){
|
||||
const renameItem=document.createElement('div');
|
||||
renameItem.textContent='Rename';
|
||||
renameItem.style.cssText='padding:7px 14px;cursor:pointer;font-size:13px;color:var(--text);';
|
||||
renameItem.onmouseenter=()=>renameItem.style.background='var(--hover)';
|
||||
renameItem.onmouseenter=()=>renameItem.style.background='var(--hover-bg)';
|
||||
renameItem.onmouseleave=()=>renameItem.style.background='';
|
||||
renameItem.onclick=()=>{menu.remove();_startProjectRename(proj,chip);};
|
||||
menu.appendChild(renameItem);
|
||||
@@ -2913,7 +2913,7 @@ function _showProjectContextMenu(e, proj, chip){
|
||||
const delItem=document.createElement('div');
|
||||
delItem.textContent='Delete';
|
||||
delItem.style.cssText='padding:7px 14px;cursor:pointer;font-size:13px;color:var(--error,#e94560);';
|
||||
delItem.onmouseenter=()=>delItem.style.background='var(--hover)';
|
||||
delItem.onmouseenter=()=>delItem.style.background='var(--hover-bg)';
|
||||
delItem.onmouseleave=()=>delItem.style.background='';
|
||||
delItem.onclick=()=>{menu.remove();_confirmDeleteProject(proj);};
|
||||
menu.appendChild(delItem);
|
||||
|
||||
+22
-2
@@ -660,13 +660,27 @@
|
||||
.has-tooltip:hover::after,.has-tooltip:focus-visible::after{opacity:1;transition-delay:.15s;}
|
||||
/* For bottom-positioned tooltips (panel header buttons, non-rail elements) */
|
||||
.has-tooltip--bottom::after{left:50%;top:auto;bottom:auto;transform:translateX(-50%);top:calc(100% + 8px);}
|
||||
/* For bottom-positioned tooltips on a trigger that sits flush with its
|
||||
container's right edge — anchors the tooltip's RIGHT edge to the trigger
|
||||
so the label extends inward (to the left) instead of overflowing past the
|
||||
panel edge. Used for the `+` New conversation button at the right of the
|
||||
chat panel header. Pairs with `--bottom`; do not apply both. */
|
||||
.has-tooltip--bottom-right::after{left:auto;right:0;top:calc(100% + 8px);bottom:auto;transform:none;}
|
||||
/* For right-edge elements (e.g. send button) — tooltip flips to the LEFT
|
||||
of the trigger so it doesn't extend past the viewport edge. */
|
||||
.has-tooltip--left::after{left:auto;right:calc(100% + 8px);top:50%;transform:translateY(-50%);}
|
||||
@media(prefers-reduced-motion:reduce){.has-tooltip::after{transition:none;transition-delay:0s;}}
|
||||
.rail-spacer{flex:1;min-height:8px;}
|
||||
.rail .nav-tab{flex:0 0 auto;padding:0;font-size:inherit;border-bottom:none;overflow:visible;}
|
||||
.rail .nav-tab:hover::after{content:none;}
|
||||
/* Note: previously this block had `.rail .nav-tab:hover::after { content: none }`
|
||||
to suppress the legacy `.nav-tab:hover::after { content: attr(data-label) }`
|
||||
tooltip (line ~681 below) on the desktop rail. After v0.51.17 migrated the
|
||||
rail to the custom `.has-tooltip` system, that suppression rule survived and
|
||||
blocked the new tooltips because `.rail .nav-tab:hover::after` (specificity
|
||||
0,3,1) outweighs `.has-tooltip:hover::after` (0,2,1) and `content:none`
|
||||
removes the pseudo-element entirely. Solution: scope the legacy
|
||||
`.nav-tab:hover::after` data-label tooltip to `.sidebar-nav` (mobile) only
|
||||
(see line ~681). The rail rule is no longer needed. */
|
||||
.rail .nav-tab.active::before{content:'';position:absolute;left:-6px;top:50%;bottom:auto;transform:translateY(-50%);width:3px;height:16px;background:var(--accent);border-radius:0 2px 2px 0;}
|
||||
.dashboard-link{position:relative;}
|
||||
.dashboard-link-visible{display:flex!important;}
|
||||
@@ -678,7 +692,13 @@
|
||||
.sidebar-nav{display:flex;border-bottom:1px solid var(--border);flex-shrink:0;padding:6px 8px 0;gap:2px;}
|
||||
.nav-tab{flex:1;padding:10px 4px 8px;font-size:20px;text-align:center;cursor:pointer;color:var(--muted);border:none;background:none;transition:color .15s;border-bottom:2px solid transparent;white-space:nowrap;overflow:hidden;position:relative;display:flex;align-items:center;justify-content:center;}
|
||||
.nav-tab:hover{color:var(--text);}
|
||||
.nav-tab:hover::after{content:attr(data-label);position:absolute;bottom:calc(100% + 8px);left:50%;transform:translateX(-50%);background:var(--surface);border:1px solid var(--accent-bg-strong);color:var(--accent-text);font-size:12px;font-weight:700;letter-spacing:.02em;padding:6px 12px;border-radius:8px;white-space:nowrap;pointer-events:none;z-index:50;box-shadow:0 4px 12px rgba(0,0,0,.3);}
|
||||
/* Legacy hover-tooltip — kept for the mobile `.sidebar-nav` only, where it
|
||||
positions ABOVE the trigger (the bar is at the top of the sidebar so a
|
||||
bottom-positioned tooltip would sink into the panel content). The desktop
|
||||
`.rail` buttons opt out of this rule so the `.has-tooltip` system can run
|
||||
unobstructed; rail buttons carry no `data-label`, so an unscoped rule
|
||||
would render an empty styled box on hover. */
|
||||
.sidebar-nav .nav-tab:hover::after{content:attr(data-label);position:absolute;bottom:calc(100% + 8px);left:50%;transform:translateX(-50%);background:var(--surface);border:1px solid var(--accent-bg-strong);color:var(--accent-text);font-size:12px;font-weight:700;letter-spacing:.02em;padding:6px 12px;border-radius:8px;white-space:nowrap;pointer-events:none;z-index:50;box-shadow:0 4px 12px rgba(0,0,0,.3);}
|
||||
.nav-tab.active{color:var(--accent-text);}
|
||||
.nav-tab.active::before{content:'';position:absolute;bottom:0;left:50%;transform:translateX(-50%);width:20px;height:2px;background:var(--accent);border-radius:2px 2px 0 0;}
|
||||
/* Panel content areas (swapped by tab) */
|
||||
|
||||
+42
-7
@@ -3045,7 +3045,11 @@ function showPromptDialog(opts={}){
|
||||
if(desc) desc.textContent=opts.message||'';
|
||||
if(input){
|
||||
input.type=opts.inputType||'text';input.style.display='';
|
||||
input.value=opts.value||'';input.placeholder=opts.placeholder||'';
|
||||
// Pre-fill: prefer `value`, accept `defaultValue` as alias for callers that
|
||||
// mirror the standard HTMLInputElement.defaultValue naming. Both empty →
|
||||
// blank field (the default rename-from-scratch flow stays unchanged).
|
||||
const prefill=(opts.value!=null?opts.value:(opts.defaultValue!=null?opts.defaultValue:''));
|
||||
input.value=prefill;input.placeholder=opts.placeholder||'';
|
||||
input.autocomplete='off';input.spellcheck=false;
|
||||
}
|
||||
if(cancelBtn) cancelBtn.textContent=opts.cancelLabel||t('cancel');
|
||||
@@ -3054,7 +3058,27 @@ function showPromptDialog(opts={}){
|
||||
if(overlay){overlay.style.display='flex';overlay.setAttribute('aria-hidden','false');}
|
||||
return new Promise(resolve=>{
|
||||
APP_DIALOG.resolve=resolve;
|
||||
setTimeout(()=>{if(input&&input.style.display!=='none')input.focus();else if(confirmBtn)confirmBtn.focus();},0);
|
||||
setTimeout(()=>{
|
||||
if(input&&input.style.display!=='none'){
|
||||
input.focus();
|
||||
// Selection behavior on focus:
|
||||
// selectStem:true → select everything before the LAST '.' (e.g. for
|
||||
// 'report.txt' selects 'report' so a user can retype the basename
|
||||
// without losing the extension; matches macOS Finder rename UX).
|
||||
// Falls back to selecting the full value when there's no '.' or
|
||||
// the dot is at index 0 ('.gitignore' → full select).
|
||||
// selectAll:true → select the entire prefilled value.
|
||||
// default → caret at end (current behavior).
|
||||
const v=input.value||'';
|
||||
if(opts.selectStem && v){
|
||||
const dot=v.lastIndexOf('.');
|
||||
if(dot>0) input.setSelectionRange(0,dot);
|
||||
else input.select();
|
||||
} else if(opts.selectAll && v){
|
||||
input.select();
|
||||
}
|
||||
} else if(confirmBtn) confirmBtn.focus();
|
||||
},0);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -6225,7 +6249,7 @@ function _showFileContextMenu(e, item){
|
||||
const renameItem=document.createElement('div');
|
||||
renameItem.textContent=t('rename_title');
|
||||
renameItem.style.cssText='padding:7px 14px;cursor:pointer;font-size:13px;color:var(--text);';
|
||||
renameItem.onmouseenter=()=>renameItem.style.background='var(--hover)';
|
||||
renameItem.onmouseenter=()=>renameItem.style.background='var(--hover-bg)';
|
||||
renameItem.onmouseleave=()=>renameItem.style.background='';
|
||||
renameItem.onclick=()=>{menu.remove();_inlineRenameFileItem(item);};
|
||||
menu.appendChild(renameItem);
|
||||
@@ -6234,7 +6258,7 @@ function _showFileContextMenu(e, item){
|
||||
const revealItem=document.createElement('div');
|
||||
revealItem.textContent=t('reveal_in_finder');
|
||||
revealItem.style.cssText='padding:7px 14px;cursor:pointer;font-size:13px;color:var(--text);';
|
||||
revealItem.onmouseenter=()=>revealItem.style.background='var(--hover)';
|
||||
revealItem.onmouseenter=()=>revealItem.style.background='var(--hover-bg)';
|
||||
revealItem.onmouseleave=()=>revealItem.style.background='';
|
||||
revealItem.onclick=async()=>{menu.remove();try{await api('/api/file/reveal',{method:'POST',body:JSON.stringify({session_id:S.session.session_id,path:item.path})});}catch(err){showToast(t('reveal_failed')+(err.message||err));}};
|
||||
menu.appendChild(revealItem);
|
||||
@@ -6247,7 +6271,7 @@ function _showFileContextMenu(e, item){
|
||||
const copyPathItem=document.createElement('div');
|
||||
copyPathItem.textContent=t('copy_file_path');
|
||||
copyPathItem.style.cssText='padding:7px 14px;cursor:pointer;font-size:13px;color:var(--text);';
|
||||
copyPathItem.onmouseenter=()=>copyPathItem.style.background='var(--hover)';
|
||||
copyPathItem.onmouseenter=()=>copyPathItem.style.background='var(--hover-bg)';
|
||||
copyPathItem.onmouseleave=()=>copyPathItem.style.background='';
|
||||
copyPathItem.onclick=async()=>{
|
||||
menu.remove();
|
||||
@@ -6286,7 +6310,7 @@ function _showFileContextMenu(e, item){
|
||||
const delItem=document.createElement('div');
|
||||
delItem.textContent=t('delete_title');
|
||||
delItem.style.cssText='padding:7px 14px;cursor:pointer;font-size:13px;color:var(--error,#e94560);';
|
||||
delItem.onmouseenter=()=>delItem.style.background='var(--hover)';
|
||||
delItem.onmouseenter=()=>delItem.style.background='var(--hover-bg)';
|
||||
delItem.onmouseleave=()=>delItem.style.background='';
|
||||
delItem.onclick=()=>{menu.remove();if(item.type==='dir')deleteWorkspaceDir(item.path,item.name);else deleteWorkspaceFile(item.path,item.name);};
|
||||
menu.appendChild(delItem);
|
||||
@@ -6298,7 +6322,18 @@ function _showFileContextMenu(e, item){
|
||||
|
||||
async function _inlineRenameFileItem(item){
|
||||
if(!S.session)return;
|
||||
const newName=await showPromptDialog({message:t('rename_prompt'),defaultValue:item.name,placeholder:item.name,confirmLabel:t('rename_title')});
|
||||
// Pre-fill the input with the current name and select just the stem
|
||||
// (everything before the last '.') so the user can immediately retype the
|
||||
// basename while preserving the extension — matches macOS Finder. For
|
||||
// directories or names with no '.', the helper selects the full value.
|
||||
// `selectStem` also handles dotfiles ('.gitignore') by full-selecting.
|
||||
const newName=await showPromptDialog({
|
||||
message:t('rename_prompt'),
|
||||
value:item.name,
|
||||
confirmLabel:t('rename_title'),
|
||||
selectStem:item.type!=='dir',
|
||||
selectAll:item.type==='dir'
|
||||
});
|
||||
if(!newName||newName===item.name)return;
|
||||
try{
|
||||
await api('/api/file/rename',{method:'POST',body:JSON.stringify({session_id:S.session.session_id,path:item.path,new_name:newName})});
|
||||
|
||||
@@ -350,5 +350,173 @@ class TestI18NTooltipSync(unittest.TestCase):
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Rail tooltip cascade regression (post-v0.51.17 follow-up)
|
||||
# ---------------------------------------------------------------------------
|
||||
class RailTooltipCascadeTests(unittest.TestCase):
|
||||
"""Pin the cascade fix that lets `.has-tooltip` work on `.rail .nav-tab`.
|
||||
|
||||
Background: the legacy `.nav-tab:hover::after { content: attr(data-label) }`
|
||||
rule was paired with a `.rail .nav-tab:hover::after { content: none }` rule
|
||||
that suppressed it on the desktop rail. After v0.51.17 migrated rail icons
|
||||
to `.has-tooltip`, the suppression rule's specificity (0,3,1) outweighed
|
||||
`.has-tooltip:hover::after` (0,2,1), and `content: none` removes the
|
||||
pseudo-element entirely — so rail tooltips never appeared. Fix: scope the
|
||||
legacy `data-label` tooltip to `.sidebar-nav .nav-tab` only and drop the
|
||||
rail suppression rule.
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
self.css = _read(STYLE_CSS)
|
||||
|
||||
def test_rail_nav_tab_hover_after_killer_is_gone(self):
|
||||
"""The `.rail .nav-tab:hover::after { content: none }` rule MUST NOT
|
||||
exist — it kills the `.has-tooltip` pseudo-element on rail buttons."""
|
||||
# Strip CSS comments first so the test doesn't false-positive on the
|
||||
# explanatory note left in place after the rule's removal.
|
||||
css_no_comments = re.sub(r"/\*.*?\*/", "", self.css, flags=re.DOTALL)
|
||||
pattern = re.compile(
|
||||
r"\.rail\s+\.nav-tab:hover:{1,2}after\s*\{[^}]*content\s*:\s*none\s*[;}]",
|
||||
re.DOTALL,
|
||||
)
|
||||
match = pattern.search(css_no_comments)
|
||||
self.assertIsNone(
|
||||
match,
|
||||
f"Found re-added killer rule that nukes rail tooltips: {match.group(0)[:120] if match else ''}",
|
||||
)
|
||||
|
||||
def test_legacy_data_label_hover_is_scoped_to_sidebar_nav(self):
|
||||
"""The legacy `data-label` hover tooltip must be scoped to
|
||||
`.sidebar-nav .nav-tab` — otherwise it fires on rail buttons (which
|
||||
carry no data-label) and renders an empty styled box on hover."""
|
||||
css_no_comments = re.sub(r"/\*.*?\*/", "", self.css, flags=re.DOTALL)
|
||||
# The unscoped bug form: `.nav-tab:hover::after { content: attr(data-label) }`
|
||||
# at the START of a selector (i.e. after `}` or whitespace+nothing-else).
|
||||
# Walk every rule whose selector ends with `.nav-tab:hover::after` and
|
||||
# check the prefix that comes before `.nav-tab`. If the prefix is empty
|
||||
# or pure whitespace, the rule is unscoped.
|
||||
for m in re.finditer(
|
||||
r"([^{}]*?)\.nav-tab:hover:{1,2}after\s*\{([^}]*content\s*:\s*attr\(data-label\)[^}]*)\}",
|
||||
css_no_comments,
|
||||
re.DOTALL,
|
||||
):
|
||||
prefix = m.group(1)
|
||||
# If the prefix (back to the previous `}` or `;`) is empty or pure
|
||||
# whitespace, this is the unscoped bug form.
|
||||
# Trim to the part after the last selector-list separator.
|
||||
last_sep = max(prefix.rfind("}"), prefix.rfind("\n"), prefix.rfind(","))
|
||||
scope_text = prefix[last_sep + 1:].strip() if last_sep >= 0 else prefix.strip()
|
||||
self.assertTrue(
|
||||
scope_text,
|
||||
"Found unscoped `.nav-tab:hover::after { content: attr(data-label) }` "
|
||||
"rule. Must be `.sidebar-nav .nav-tab:hover::after` so it does not "
|
||||
"fire on rail buttons that carry no data-label.",
|
||||
)
|
||||
|
||||
# Affirmative: the scoped form must exist.
|
||||
good_pattern = re.compile(
|
||||
r"\.sidebar-nav\s+\.nav-tab:hover:{1,2}after\s*\{[^}]*content\s*:\s*attr\(data-label\)",
|
||||
re.DOTALL,
|
||||
)
|
||||
self.assertIsNotNone(
|
||||
good_pattern.search(css_no_comments),
|
||||
"Expected `.sidebar-nav .nav-tab:hover::after { content: attr(data-label); ... }` "
|
||||
"rule (mobile sidebar fallback tooltip). It went missing.",
|
||||
)
|
||||
|
||||
def test_all_rail_buttons_carry_has_tooltip(self):
|
||||
"""Every `.rail-btn.nav-tab` button must carry `class="has-tooltip"` and
|
||||
a non-empty `data-tooltip` attribute. Otherwise the rail tooltip is
|
||||
invisible regardless of the cascade fix above."""
|
||||
html = _read(INDEX_HTML)
|
||||
# Find the rail block: <nav class="rail" ...> ... </nav>
|
||||
rail_match = re.search(
|
||||
r'<nav class="rail"[^>]*>(.*?)</nav>',
|
||||
html,
|
||||
re.DOTALL,
|
||||
)
|
||||
self.assertIsNotNone(rail_match, "Could not locate <nav class='rail'> in index.html")
|
||||
rail_block = rail_match.group(1)
|
||||
|
||||
rail_btn_count = 0
|
||||
missing = []
|
||||
for m in re.finditer(r'<button\b([^>]*?)>', rail_block):
|
||||
attrs = m.group(1)
|
||||
if 'rail-btn' not in attrs:
|
||||
continue
|
||||
rail_btn_count += 1
|
||||
if 'has-tooltip' not in attrs:
|
||||
missing.append(('class missing has-tooltip', attrs[:120]))
|
||||
continue
|
||||
tooltip_attr = re.search(r'data-tooltip="([^"]*)"', attrs)
|
||||
if not tooltip_attr or not tooltip_attr.group(1).strip():
|
||||
missing.append(('missing or empty data-tooltip', attrs[:120]))
|
||||
|
||||
self.assertGreaterEqual(
|
||||
rail_btn_count, 10,
|
||||
f"Expected ≥10 rail buttons (found {rail_btn_count}). Test selector wrong?",
|
||||
)
|
||||
self.assertEqual(
|
||||
missing, [],
|
||||
f"Rail buttons without working tooltip markup:\n " +
|
||||
"\n ".join(f"{reason}: {attrs}" for reason, attrs in missing),
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# `--bottom-right` variant: anchors tooltip's RIGHT edge to a trigger that sits
|
||||
# flush with its container's right edge, so the label extends inward instead of
|
||||
# overflowing past the panel edge. Used by `#btnNewChat`.
|
||||
# ---------------------------------------------------------------------------
|
||||
class BottomRightTooltipVariantTests(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self.css = _read(STYLE_CSS)
|
||||
self.html = _read(INDEX_HTML)
|
||||
|
||||
def test_bottom_right_variant_defined(self):
|
||||
"""`.has-tooltip--bottom-right::after` must exist and right-anchor the
|
||||
tooltip (`right: 0` and no `transform: translateX`)."""
|
||||
rule = re.search(
|
||||
r"\.has-tooltip--bottom-right:{1,2}after\s*\{([^}]*)\}",
|
||||
self.css,
|
||||
re.DOTALL,
|
||||
)
|
||||
self.assertIsNotNone(rule, "`.has-tooltip--bottom-right::after` rule missing")
|
||||
body = rule.group(1)
|
||||
# Must anchor right edge.
|
||||
self.assertRegex(body, r"right\s*:\s*0",
|
||||
"--bottom-right variant must set right:0")
|
||||
# Must clear the inherited `left:` so it doesn't fight with the base rule.
|
||||
self.assertRegex(body, r"left\s*:\s*auto",
|
||||
"--bottom-right variant must clear left:auto")
|
||||
# Must clear the inherited transform (otherwise translateX(-50%) shifts it).
|
||||
self.assertRegex(body, r"transform\s*:\s*none",
|
||||
"--bottom-right variant must reset transform:none")
|
||||
|
||||
def test_btn_new_chat_uses_bottom_right_variant(self):
|
||||
"""`#btnNewChat` sits flush with the chat-panel right edge; its tooltip
|
||||
previously overflowed (with `--bottom`, half clips past the panel).
|
||||
Must now use `--bottom-right`, NOT `--bottom`."""
|
||||
match = re.search(
|
||||
r'<button[^>]*\bid="btnNewChat"[^>]*>',
|
||||
self.html,
|
||||
)
|
||||
self.assertIsNotNone(match, "Could not find #btnNewChat button")
|
||||
attrs = match.group(0)
|
||||
self.assertIn(
|
||||
"has-tooltip--bottom-right",
|
||||
attrs,
|
||||
"#btnNewChat must carry has-tooltip--bottom-right so its tooltip "
|
||||
"doesn't overflow the chat-panel right edge.",
|
||||
)
|
||||
# Must NOT also carry the old --bottom (would conflict).
|
||||
self.assertNotRegex(
|
||||
attrs,
|
||||
r'has-tooltip--bottom(?!-)',
|
||||
"#btnNewChat carries both --bottom and --bottom-right; pick one. "
|
||||
"The plain --bottom variant centers on left:50% and overflows.",
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
@@ -0,0 +1,263 @@
|
||||
"""
|
||||
Workspace context-menu hover and rename-dialog pre-fill regressions.
|
||||
|
||||
Two distinct bugs that were both shipped at the same time and only caught when
|
||||
a user dogfooded the workspace panel:
|
||||
|
||||
(a) Workspace + session-list right-click context menu items had no visible
|
||||
hover state because they wrote `style.background = 'var(--hover)'`. The
|
||||
custom property `--hover` is undefined anywhere in the codebase. An
|
||||
undefined `var()` falls back to the property's initial value (transparent
|
||||
for `background`), so the hover state silently no-op'd. The defined
|
||||
variable is `--hover-bg` (`rgba(255,255,255,.06)`), used by every other
|
||||
hover state in the app — there's a one-letter typo that ate every
|
||||
context-menu hover.
|
||||
|
||||
(b) Right-click → Rename did not pre-fill the input with the current filename.
|
||||
`_inlineRenameFileItem` passed `defaultValue: item.name` to
|
||||
`showPromptDialog`, but the dialog's input setter reads `opts.value` only.
|
||||
The `defaultValue` parameter was silently dropped; only the placeholder
|
||||
showed (the "ghost" name the user described).
|
||||
|
||||
Run: /root/hermes-agent/venv/bin/python -m pytest tests/test_workspace_context_menu_and_rename.py -v
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import re
|
||||
import unittest
|
||||
|
||||
|
||||
BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
|
||||
UI_JS = os.path.join(BASE_DIR, "static", "ui.js")
|
||||
SESSIONS_JS = os.path.join(BASE_DIR, "static", "sessions.js")
|
||||
|
||||
|
||||
def _read(path: str) -> str:
|
||||
with open(path, encoding="utf-8") as fh:
|
||||
return fh.read()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# (a) Context-menu hover background — `--hover` was undefined; must use --hover-bg
|
||||
# ---------------------------------------------------------------------------
|
||||
class ContextMenuHoverBackgroundTests(unittest.TestCase):
|
||||
"""Pin: no JS code path may set `style.background = 'var(--hover)'`.
|
||||
|
||||
The variable is undefined; the resolved value is `transparent`, which gives
|
||||
no visible hover feedback. Use `var(--hover-bg)` (the actual variable used
|
||||
by every other hover state in the codebase).
|
||||
"""
|
||||
|
||||
def test_no_var_hover_in_ui_js(self):
|
||||
src = _read(UI_JS)
|
||||
# Match `var(--hover)` but NOT `var(--hover-bg)` / `var(--hover-2)` etc.
|
||||
# Negative lookahead handles the `-` case; we also bar `_` and word chars.
|
||||
bad = re.findall(r"var\(--hover\)(?![\w-])", src)
|
||||
self.assertEqual(
|
||||
bad, [],
|
||||
f"Found {len(bad)} `var(--hover)` reference(s) in static/ui.js. "
|
||||
"The variable `--hover` is undefined; this resolves to `transparent` "
|
||||
"and breaks visible hover state. Use `var(--hover-bg)` instead.",
|
||||
)
|
||||
|
||||
def test_no_var_hover_in_sessions_js(self):
|
||||
src = _read(SESSIONS_JS)
|
||||
bad = re.findall(r"var\(--hover\)(?![\w-])", src)
|
||||
self.assertEqual(
|
||||
bad, [],
|
||||
f"Found {len(bad)} `var(--hover)` reference(s) in static/sessions.js. "
|
||||
"Use `var(--hover-bg)` (the defined variable).",
|
||||
)
|
||||
|
||||
def test_file_context_menu_uses_var_hover_bg(self):
|
||||
"""Affirmative pin on the file context menu in ui.js — every menu item
|
||||
builder (Rename, Reveal, Copy path, Delete) must use `var(--hover-bg)`."""
|
||||
src = _read(UI_JS)
|
||||
fn_match = re.search(
|
||||
r"function\s+_showFileContextMenu\b[^{]*\{",
|
||||
src,
|
||||
)
|
||||
self.assertIsNotNone(fn_match, "Could not find _showFileContextMenu()")
|
||||
# Slice from start of function until the matching closing brace at
|
||||
# column 0 (next top-level function). Cheap brace-balance.
|
||||
start = fn_match.start()
|
||||
depth = 0
|
||||
end = start
|
||||
for i, ch in enumerate(src[start:], start=start):
|
||||
if ch == "{":
|
||||
depth += 1
|
||||
elif ch == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
end = i + 1
|
||||
break
|
||||
body = src[start:end]
|
||||
# Expect at least 4 hover assignments (one per menu item).
|
||||
hits = re.findall(r"\.style\.background\s*=\s*['\"]var\(--hover-bg\)['\"]", body)
|
||||
self.assertGreaterEqual(
|
||||
len(hits), 4,
|
||||
f"Expected ≥4 menu items to set background to var(--hover-bg) "
|
||||
f"(Rename, Reveal, Copy path, Delete). Found {len(hits)}.",
|
||||
)
|
||||
|
||||
def test_session_context_menu_uses_var_hover_bg(self):
|
||||
"""Affirmative pin on the project chip context menu in sessions.js."""
|
||||
src = _read(SESSIONS_JS)
|
||||
fn_match = re.search(
|
||||
r"function\s+_showProjectContextMenu\b[^{]*\{",
|
||||
src,
|
||||
)
|
||||
self.assertIsNotNone(fn_match, "Could not find _showProjectContextMenu()")
|
||||
start = fn_match.start()
|
||||
depth = 0
|
||||
end = start
|
||||
for i, ch in enumerate(src[start:], start=start):
|
||||
if ch == "{":
|
||||
depth += 1
|
||||
elif ch == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
end = i + 1
|
||||
break
|
||||
body = src[start:end]
|
||||
hits = re.findall(r"\.style\.background\s*=\s*['\"]var\(--hover-bg\)['\"]", body)
|
||||
self.assertGreaterEqual(
|
||||
len(hits), 2,
|
||||
f"Expected ≥2 menu items to set background to var(--hover-bg) "
|
||||
f"in _showProjectContextMenu. Found {len(hits)}.",
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# (b) showPromptDialog pre-fill: must accept both `value` and `defaultValue`
|
||||
# ---------------------------------------------------------------------------
|
||||
class ShowPromptDialogPrefillTests(unittest.TestCase):
|
||||
"""The rename dialog must pre-fill with the current filename (matches
|
||||
every native file manager) AND the dialog must accept `defaultValue` as an
|
||||
alias for `value` — the typo that caused the original bug is too easy to
|
||||
repeat with no API alias.
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
self.src = _read(UI_JS)
|
||||
|
||||
def _slice_show_prompt_dialog(self) -> str:
|
||||
"""Return the body of `showPromptDialog(opts={}){ ... }` as a string."""
|
||||
# Anchor: the `function showPromptDialog` keyword. Skip past the
|
||||
# parameter list (which contains `opts={}` — its `{}` would fool a naive
|
||||
# brace counter), then balance braces from the function-body opener.
|
||||
kw = re.search(r"function\s+showPromptDialog\b", self.src)
|
||||
self.assertIsNotNone(kw, "Could not find showPromptDialog()")
|
||||
# Find the parameter-list parens — skip over them by parens balance.
|
||||
i = kw.end()
|
||||
# advance to the opening '('
|
||||
while i < len(self.src) and self.src[i] != "(":
|
||||
i += 1
|
||||
self.assertLess(i, len(self.src), "showPromptDialog: no opening paren")
|
||||
depth = 0
|
||||
while i < len(self.src):
|
||||
ch = self.src[i]
|
||||
if ch == "(":
|
||||
depth += 1
|
||||
elif ch == ")":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
i += 1
|
||||
break
|
||||
i += 1
|
||||
# Now skip whitespace to the function-body '{'.
|
||||
while i < len(self.src) and self.src[i] not in "{":
|
||||
i += 1
|
||||
self.assertLess(i, len(self.src), "showPromptDialog: no function-body brace")
|
||||
start = i
|
||||
depth = 0
|
||||
end = start
|
||||
for j, ch in enumerate(self.src[start:], start=start):
|
||||
if ch == "{":
|
||||
depth += 1
|
||||
elif ch == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
end = j + 1
|
||||
break
|
||||
return self.src[start:end]
|
||||
|
||||
def test_show_prompt_dialog_accepts_default_value_alias(self):
|
||||
body = self._slice_show_prompt_dialog()
|
||||
# Must reference `opts.defaultValue` somewhere — the alias was the
|
||||
# backward-compatibility fix so future typos don't cause silent drops.
|
||||
self.assertIn(
|
||||
"opts.defaultValue", body,
|
||||
"showPromptDialog must accept `defaultValue` as an alias for "
|
||||
"`value` so callers using the standard HTMLInputElement.defaultValue "
|
||||
"param name pre-fill correctly (regression protection).",
|
||||
)
|
||||
# Must still reference `opts.value` — the canonical param.
|
||||
self.assertIn("opts.value", body)
|
||||
|
||||
def test_show_prompt_dialog_supports_select_stem(self):
|
||||
"""Stem selection (everything before the last '.') is what makes
|
||||
rename-with-pre-fill actually fast — user can immediately type the new
|
||||
basename without losing the extension. Without this, pre-fill plus a
|
||||
full-string select would force the user to type the extension every
|
||||
time."""
|
||||
body = self._slice_show_prompt_dialog()
|
||||
self.assertIn(
|
||||
"selectStem", body,
|
||||
"showPromptDialog should support `selectStem:true` to select the "
|
||||
"filename portion before the last '.' on focus (Finder-style "
|
||||
"rename UX).",
|
||||
)
|
||||
# Pin the actual stem-selection mechanic — must use lastIndexOf('.')
|
||||
# and setSelectionRange. Anything else is the wrong selection rule.
|
||||
self.assertRegex(
|
||||
body, r"lastIndexOf\(\s*['\"]\.['\"]\s*\)",
|
||||
"selectStem must use lastIndexOf('.') so 'a.b.c.d' selects 'a.b.c'.",
|
||||
)
|
||||
self.assertRegex(
|
||||
body, r"setSelectionRange\s*\(\s*0\s*,",
|
||||
"selectStem must use setSelectionRange(0, dot) to select the stem.",
|
||||
)
|
||||
|
||||
def test_inline_rename_uses_value_and_select_stem(self):
|
||||
"""The rename caller must (a) pre-fill the current name via `value:`
|
||||
and (b) ask for `selectStem:true` on files (so the extension survives)
|
||||
— these are the two legs of the user-visible fix."""
|
||||
m = re.search(
|
||||
r"async\s+function\s+_inlineRenameFileItem\b[^{]*\{",
|
||||
self.src,
|
||||
)
|
||||
self.assertIsNotNone(m, "Could not find _inlineRenameFileItem()")
|
||||
start = m.start()
|
||||
depth = 0
|
||||
end = start
|
||||
for i, ch in enumerate(self.src[start:], start=start):
|
||||
if ch == "{":
|
||||
depth += 1
|
||||
elif ch == "}":
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
end = i + 1
|
||||
break
|
||||
body = self.src[start:end]
|
||||
# Must pass value:item.name (not defaultValue:item.name — the original bug).
|
||||
self.assertRegex(
|
||||
body,
|
||||
r"value\s*:\s*item\.name",
|
||||
"_inlineRenameFileItem must pass `value:item.name` to pre-fill "
|
||||
"the dialog input. (The original `defaultValue:item.name` was "
|
||||
"silently dropped because the dialog reads `opts.value`.)",
|
||||
)
|
||||
# Must opt into selectStem for files (not directories).
|
||||
self.assertIn(
|
||||
"selectStem", body,
|
||||
"_inlineRenameFileItem must pass selectStem:... so renaming "
|
||||
"'report.txt' selects 'report' and the user can immediately type "
|
||||
"the new basename while preserving the extension.",
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user