From b49c3cbd433367339b9e2c1629afa040b2b2a0c6 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Thu, 7 May 2026 06:14:03 +0000 Subject: [PATCH] fix(ux): rail tooltips, +new-conversation clipping, context-menu hover, rename pre-fill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four small UX bugs Nathan caught while dogfooding the v0.51.17 release on desktop. All independently reproduced with browser_console + browser_vision on a fresh worktree before fixing. (1) **Left-rail icon tooltips never appeared.** The rail was migrated to the new `.has-tooltip` system in #1782, but the legacy suppression rule `.rail .nav-tab:hover::after { content: none }` survived the migration. Its specificity (0,3,1) outweighs `.has-tooltip:hover::after` (0,2,1), and `content: none` removes the pseudo-element entirely on hover — so the new tooltip system silently no-op'd on every rail icon. Fix: drop the suppression rule and scope the legacy `data-label` tooltip to `.sidebar-nav .nav-tab` (mobile) only, so it doesn't fire on rail buttons that carry no `data-label` (which would render an empty styled box). (2) **`+ New conversation` tooltip clipped at panel right edge.** The button sits flush with the chat panel's right edge but used `--bottom` which centers the tooltip on `left:50%` — half the label overflowed past the panel edge ("New convers..."). New `.has-tooltip--bottom-right` variant anchors the tooltip's RIGHT edge to the trigger so the label extends inward. Reusable for any future right-edge panel-head button. (3) **Workspace right-click menu items had no hover state.** The five sites in `_showFileContextMenu` (Rename / Reveal / Copy path / Delete) and two in `_showProjectContextMenu` set `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`) → no visible hover feedback. The defined variable is `--hover-bg` (`rgba(255,255,255,.06)`), already used by every other hover state in the app. One-letter typo, seven sites. (4) **Rename dialog didn't pre-fill the current filename.** The caller (`_inlineRenameFileItem`) passed `defaultValue: item.name` to `showPromptDialog`, but the dialog's input setter reads `opts.value` only — the param name was silently dropped, leaving only the placeholder visible (Nathan called it the "ghost name"). Fixed two ways for defense-in-depth: - Caller switched to canonical `value: item.name`. - Dialog now also accepts `defaultValue` as an alias for `value`, so future typos using the standard `HTMLInputElement.defaultValue` param name don't repeat the bug. Plus: added `selectStem:true` opt that selects the stem before the last `.` on focus (Finder-style: `report.txt` → selects `report`, extension preserved). Edge cases verified live: directories full-select, `.gitignore` full-selects (dot at index 0), `noextension` full-selects, `a.b.c.d` selects `a.b.c`. ## Tests +12 new regression tests, +5 net (existing test_css_tooltips suite gained 5 class-based tests; new tests/test_workspace_context_menu_and_rename.py file adds 7 more). Total: 4728 passed (was 4723 in v0.51.17), 4 skipped, 3 xpassed, 0 failed in 141s. - `RailTooltipCascadeTests` — pins the killer rule's absence (with comment stripping so the explanatory note doesn't false-positive), pins the scoped `.sidebar-nav .nav-tab` form, walks every rail button to confirm `has-tooltip` + non-empty `data-tooltip`. - `BottomRightTooltipVariantTests` — pins variant existence, mechanics (`right:0`, `left:auto`, `transform:none`), and `#btnNewChat` adoption (with mutual-exclusion check that it doesn't carry both `--bottom` and `--bottom-right`). - `ContextMenuHoverBackgroundTests` — `var(--hover)` may not appear in ui.js or sessions.js (the bug shape); affirmative pin that `_showFileContextMenu` sets ≥4 items to `var(--hover-bg)` and `_showProjectContextMenu` ≥2. - `ShowPromptDialogPrefillTests` — pins both `opts.value` and `opts.defaultValue` references; pins the `selectStem` mechanic (`lastIndexOf('.')` + `setSelectionRange(0, dot)`); pins the caller's use of `value:item.name` and `selectStem`. ## Verification Live in browser at port 8789 (worktree-served): - Rail Tasks tooltip renders 8px right of the icon at the same vertical level (math: btn at y=87-123, tooltip at left=44px = 36px width + 8px gap). - New-conversation tooltip renders below + button with right edge aligned to button's right edge, extending leftward, fully visible. - Right-click → Reveal in File Manager shows `rgba(255, 255, 255, 0.035)` background on hover (the `--hover-bg` value); was `rgba(0, 0, 0, 0)` (transparent) before. - Right-click → Rename on `report.txt`: input shows `report.txt`, selectionStart=0, selectionEnd=6, selected text = "report". Edge cases: directory `docs` → full-select; `.gitignore` → full-select; `noextension` → full-select; `a.b.c.d` → selects `a.b.c`. `node -c` syntax check passes on both modified JS files. Reported by: Nathan via screenshots (rail tooltips missing, + button clipped tooltip, Workspace right-click no hover, rename dialog blank). --- static/index.html | 2 +- static/sessions.js | 4 +- static/style.css | 24 +- static/ui.js | 49 +++- tests/test_css_tooltips.py | 168 +++++++++++ .../test_workspace_context_menu_and_rename.py | 263 ++++++++++++++++++ 6 files changed, 498 insertions(+), 12 deletions(-) create mode 100644 tests/test_workspace_context_menu_and_rename.py diff --git a/static/index.html b/static/index.html index dd97d70d..2eb061ad 100644 --- a/static/index.html +++ b/static/index.html @@ -122,7 +122,7 @@
Chat
-
diff --git a/static/sessions.js b/static/sessions.js index e7892a0f..a925b1c8 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -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); diff --git a/static/style.css b/static/style.css index 9f413a34..d8d28922 100644 --- a/static/style.css +++ b/static/style.css @@ -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) */ diff --git a/static/ui.js b/static/ui.js index bc498542..ff666883 100644 --- a/static/ui.js +++ b/static/ui.js @@ -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})}); diff --git a/tests/test_css_tooltips.py b/tests/test_css_tooltips.py index 125faed0..2e31cc25 100644 --- a/tests/test_css_tooltips.py +++ b/tests/test_css_tooltips.py @@ -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: + rail_match = re.search( + r'', + html, + re.DOTALL, + ) + self.assertIsNotNone(rail_match, "Could not locate